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 treegraph
 
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

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.