Bug 13959 - ldb_tdb fails to check error return when parsing pack formats
Summary: ldb_tdb fails to check error return when parsing pack formats
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.10.3
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 13978
  Show dependency tree / graph
 
Reported: 2019-05-22 04:36 UTC by Andrew Bartlett
Modified: 2022-01-04 23:53 UTC (History)
3 users (show)

See Also:

Attachments
patch for master and 4.10 (2.28 KB, patch)
2019-05-22 04:45 UTC, Andrew Bartlett 		no flags Details
patch for 4.9 (not just a cherry-pick from master) (2.36 KB, patch)
2019-05-22 05:00 UTC, Andrew Bartlett 		no flags Details
patch for Samba 4.9 (backported/cherry-picked from master) (36.56 KB, patch)
2019-06-11 09:07 UTC, Andrew Bartlett 		gary: review+
abartlet: ci-passed+
Details
patch for Samba 4.10 (backported/cherry-picked from master) (36.51 KB, patch)
2019-06-11 09:11 UTC, Andrew Bartlett 		gary: review+
Details
patch for Samba 4.9 (backported/cherry-picked from master) V2 (36.52 KB, patch)
2019-07-16 23:57 UTC, Aaron Haslett (dead mail address) 		abartlet: review+
garming: review-
Details
patch for Samba 4.10 (backported/cherry-picked from master) V2 (36.52 KB, patch)
2019-07-16 23:57 UTC, Aaron Haslett (dead mail address) 		abartlet: review+
garming: review-
Details
patch for Samba 4.9 (backported/cherry-picked from master) V3 (36.50 KB, patch)
2019-07-31 01:22 UTC, Aaron Haslett (dead mail address) 		no flags Details
patch for Samba 4.10 (backported/cherry-picked from master) V3 (36.50 KB, patch)
2019-07-31 01:23 UTC, Aaron Haslett (dead mail address) 		no flags Details
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2019-05-22 04:36:08 UTC 
ldb_tdb silently ignores all errors from ldb_unpack() et al.

This means that new pack formats segfault old LDB versions until patched.
Comment 1 Andrew Bartlett 2019-05-22 04:45:36 UTC 
Created attachment 15177 [details]
patch for master and 4.10

This needs tests added.
Comment 2 Andrew Bartlett 2019-05-22 04:51:29 UTC 
To be clear, even without a new pack format this is a big issue, as any DB corruption is totally ignored, with the records just returning short with data not filled in. 

The best case is actually the new pack format because msg->elements is left alone, so normally NULL from ldb_msg_new().  This causes a segfault later as msg->num_elements has been filled in.
Comment 3 Andrew Bartlett 2019-05-22 05:00:33 UTC 
Created attachment 15178 [details]
patch for 4.9 (not just a cherry-pick from master)
Comment 4 Andrew Bartlett 2019-06-11 09:07:49 UTC 
Created attachment 15236 [details]
patch for Samba 4.9 (backported/cherry-picked from master)
Comment 5 Andrew Bartlett 2019-06-11 09:11:31 UTC 
Created attachment 15237 [details]
patch for Samba 4.10 (backported/cherry-picked from master)
Comment 6 Andrew Bartlett 2019-06-11 09:12:48 UTC 
Comment on attachment 15236 [details]
patch for Samba 4.9 (backported/cherry-picked from master)

Tested in this pipeine with downgradedatabase changes:
https://gitlab.com/samba-team/devel/samba/pipelines/65456993
Comment 7 Andrew Bartlett 2019-06-12 04:33:23 UTC 
Assigning to Karolin for the next releases.  Apply after 13978 which provides some support.
Comment 8 Aaron Haslett (dead mail address) 2019-07-16 23:57:00 UTC 
Created attachment 15312 [details]
patch for Samba 4.9 (backported/cherry-picked from master) V2

Fixed LDB ABI versioning for 4.9 - cherry picked and tested.
Comment 9 Aaron Haslett (dead mail address) 2019-07-16 23:57:47 UTC 
Created attachment 15313 [details]
patch for Samba 4.10 (backported/cherry-picked from master) V2

Fixed LDB ABI versioning for 4.10.6; tested.
Comment 10 Aaron Haslett (dead mail address) 2019-07-17 01:18:41 UTC 
Final decision will have to wait till Andrew's back, but in my view this should go in regardless of the status of 13978, since it fixes a segfault.
Comment 11 Andrew Bartlett 2019-07-23 22:13:48 UTC 
Yes, these need to go into the older releases to avoid segfaults when folks try and downgrade. 

Once we get past the segfaults we have something (while still cryptic) that users can search on etc.
Comment 12 Garming Sam 2019-07-30 02:53:14 UTC 
Comment on attachment 15312 [details]
patch for Samba 4.9 (backported/cherry-picked from master) V2

Commit message for the version bump no longer matches the patch, please change the message (for the 4.10 patch too).
Comment 13 Aaron Haslett (dead mail address) 2019-07-31 01:22:48 UTC 
Created attachment 15351 [details]
patch for Samba 4.9 (backported/cherry-picked from master) V3

Fixed LDB version in final commit message
Comment 14 Aaron Haslett (dead mail address) 2019-07-31 01:23:29 UTC 
Created attachment 15352 [details]
patch for Samba 4.10 (backported/cherry-picked from master) V3

Fixed LDB version in final commit message
Comment 15 Aaron Haslett (dead mail address) 2019-07-31 01:43:18 UTC 
This bug is now covered by the META bug:
https://bugzilla.samba.org/show_bug.cgi?id=14062

Please go there instead of adding more confusion to this bug.
Comment 16 Björn Jacke 2022-01-04 23:53:27 UTC 
looks like the backports for older releases had not been done - closing now.