Bug 13773 (CVE-2019-3824) - CVE-2019-3824 [SECURITY] ldb: Out of bound read in ldb_wildcard_compare
Summary: CVE-2019-3824 [SECURITY] ldb: Out of bound read in ldb_wildcard_compare
Status: RESOLVED FIXED
Alias: CVE-2019-3824
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Gary Lockyer
QA Contact: Samba QA Contact
URL: https://lists.samba.org/archive/samba...
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-01 04:09 UTC by Andrew Bartlett
Modified: 2019-04-11 21:26 UTC (History)
11 users (show)

See Also:


Attachments
patch from Lukas Slebodnik (4.25 KB, patch)
2019-02-01 04:09 UTC, Andrew Bartlett
no flags Details
Initial advisory (1.79 KB, text/plain)
2019-02-01 04:24 UTC, Andrew Bartlett
no flags Details
advisory with CVE (v1) (1.81 KB, text/plain)
2019-02-01 23:36 UTC, Andrew Bartlett
no flags Details
updated patch for master (v2) (30.60 KB, patch)
2019-02-14 03:57 UTC, Andrew Bartlett
no flags Details
Updated patch for master (v3) (41.49 KB, patch)
2019-02-19 19:46 UTC, Gary Lockyer
abartlet: review+
Details
Proposed patch for version V4.9 (41.42 KB, patch)
2019-02-20 02:24 UTC, Gary Lockyer
abartlet: review+
Details
Proposed patch for version V4.10 (144.62 KB, patch)
2019-02-20 03:29 UTC, Gary Lockyer
abartlet: review-
Details
Proposed patch for version V4.10 V1 (41.49 KB, patch)
2019-02-20 03:43 UTC, Gary Lockyer
abartlet: review+
Details
Proposed patch for V4.5 (32.82 KB, patch)
2019-02-20 20:05 UTC, Gary Lockyer
abartlet: review-
Details
Proposed patch for V4.5 V2 (10.33 KB, patch)
2019-02-20 20:30 UTC, Gary Lockyer
abartlet: review+
Details
Proposed patch fo V4.8 (41.24 KB, patch)
2019-02-20 20:51 UTC, Gary Lockyer
gary: review? (abartlet)
Details
Proposed patch for V4.7 (41.02 KB, patch)
2019-02-20 21:09 UTC, Gary Lockyer
abartlet: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2019-02-01 04:09:01 UTC
Created attachment 14814 [details]
patch from Lukas Slebodnik

From Garming:

Just looking briefly at the code, it looks like you read at least
cnk.length after the end of the memory buffer (whose length you can
modify by presumably changing the search expression to have longer
wildcard chunks).

If the pointer was pointing into the middle of database memory, maybe
you could extract details about adjacent records but it seems like the
canonicalise avoids this by effectively duplicating the value. So it
looks like it's always adjacent to arbitrary heap memory.

Presumably you can crash (LDAP server), or accidentally return records
that don't match the expression merely by chance. But all the higher
layers and ACL checks should prevent returning any data that the user
isn't privileged enough to see. I think it's only a DoS, with it mostly
affecting samba in single process mode and pre-fork before the restart
code which is in master + 4.10.
Comment 1 Andrew Bartlett 2019-02-01 04:09:48 UTC
So, this is how I plan to handle this.

For the default AD DC configuration, the default process model is -M standard in all releases.  Therefore my view is that this issue is minor (it is a self-DoS of your own forked LDAP connection) and can just be fixed in master.  

While the AD DC 'samba' binary can be run in the non-default -M prefork or -M single mode, this is pretty rare.  In those cases the DoS would take out the server. 

The real worry is Samba 4.6 and earlier, but these are unsupported.

The issue is public and pretty minor, but we patch in master as normal once the embargo is over. 

Therefore there is no release date or official Samba security release.

I'll follow the security_releases.txt in other respects however to allow Debian and vendors a chance to apply the backported patch.
Comment 2 Andrew Bartlett 2019-02-01 04:24:14 UTC
Created attachment 14815 [details]
Initial advisory
Comment 3 Andrew Bartlett 2019-02-01 23:36:17 UTC
Created attachment 14819 [details]
advisory with CVE (v1)
Comment 4 Andrew Bartlett 2019-02-14 00:53:46 UTC
Embargo is automatically lifted 9am CEST 25 Feb.  

(So I can patch this in master and propose a backport before Samba 4.10 freezes for the 4.10.0 release on 26 Feb, as our release manager does not like changes in the final week). 

While I will leave the CVE number here (to facilitate tracking for those needing to backport), this is NOT in security support, NO announcement mail will be sent.

This is the announcement for known vendors who will need to backport. 

In the next few days we will write up a more specific test and so provide an updated patch for master (which in turn can be backported to the actually impacted versions).
Comment 5 Andrew Bartlett 2019-02-14 03:57:30 UTC
Created attachment 14845 [details]
updated patch for master (v2)

A further updated patch with a specific passing/failing test will be provided tomorrow.
Comment 6 Gary Lockyer 2019-02-19 19:46:10 UTC
Created attachment 14852 [details]
Updated patch for master (v3)

Updated patch, with tests and fixes to issues found reviewing and testing the code.

The tests need to be run under valgrind to verify the fix works.

valgrind --suppressions=lib/ldb/tests/ldb_match_test.valgrind bin/ldb_match_test
Comment 7 Gary Lockyer 2019-02-20 02:24:01 UTC
Created attachment 14853 [details]
Proposed patch for version V4.9

Passes private CI
Comment 8 Andrew Bartlett 2019-02-20 02:50:53 UTC
Comment on attachment 14852 [details]
Updated patch for master (v3)

Looks good. 

In a future iteration (not to re-start the backport process) check the return values from the wildcard call in all the test cases.
Comment 9 Gary Lockyer 2019-02-20 03:29:28 UTC
Created attachment 14854 [details]
Proposed patch for version V4.10

Passes private CI.
Comment 10 Gary Lockyer 2019-02-20 03:43:34 UTC
Created attachment 14855 [details]
Proposed patch for version V4.10 V1
Comment 11 Gary Lockyer 2019-02-20 20:05:03 UTC
Created attachment 14856 [details]
Proposed patch for V4.5
Comment 12 Andrew Bartlett 2019-02-20 20:27:35 UTC
Comment on attachment 14856 [details]
Proposed patch for V4.5

Thinking hard about this, if this patch series can't ever land in a samba.org official branch, then we don't want the version number bumped as otherwise the next security release might try the same thing and have two different 1.1.28 versions in the wild.

In particular, with this patch Debian would be likely to release a 1.1.28 based on this. 

Just drop the version change patch.

Sorry!
Comment 13 Gary Lockyer 2019-02-20 20:30:27 UTC
Created attachment 14857 [details]
Proposed patch for V4.5 V2
Comment 14 Gary Lockyer 2019-02-20 20:51:00 UTC
Created attachment 14858 [details]
Proposed patch fo V4.8
Comment 15 Gary Lockyer 2019-02-20 21:09:04 UTC
Created attachment 14859 [details]
Proposed patch for V4.7
Comment 16 Mathieu Parent 2019-02-20 21:27:35 UTC
Thanks for the 4.5 v2 patch. I'm currently testing it.
Comment 17 Stefan Metzmacher 2019-02-26 16:02:41 UTC
Pushed to v4-{7,8,9,10}-test and master. The related standalone ldb releases
are also published.
Comment 18 Huzaifa Sidhpurwala 2019-02-28 05:26:58 UTC
I can find https://www.samba.org/samba/security/CVE-2019-3824.html

Any plans to publish this?
Comment 19 Andrew Bartlett 2019-04-11 21:26:22 UTC
Sorry for the late reply, as this had no significant impact on supported versions, there wasn't a Samba security announcment for this (but we tried to follow other aspects of the process to give vendors still shipping a fair chance to address it). 

We would welcome feedback on the process, as this is the first time for this particular circumstance.

Thanks!