Bug 15276 (CVE-2023-0225) - CVE-2023-0225 [SECURITY] Unprivileged user can delete dNSHostName attribute
Summary: CVE-2023-0225 [SECURITY] Unprivileged user can delete dNSHostName attribute
Status: RESOLVED FIXED
Alias: CVE-2023-0225
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.17.4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 15337
  Show dependency tree / graph
 
Reported: 2023-01-03 17:58 UTC by Ralph Böhme
Modified: 2023-04-05 03:14 UTC (History)
7 users (show)

See Also:

Attachments
test confirming the behaviour (9.03 KB, patch)
2023-01-04 08:45 UTC, Douglas Bagnall 		no flags Details
test and proposed fix for master (13.11 KB, patch)
2023-01-08 23:11 UTC, Jo Sutton 		no flags Details
Initial advisory without versions (2.03 KB, text/plain)
2023-02-22 00:35 UTC, Andrew Bartlett 		jsutton: review+
Details
Updated advisory without versions (v2) (2.05 KB, text/plain)
2023-02-22 01:21 UTC, Andrew Bartlett 		jsutton: review+
Details
Advisory v3 (2.12 KB, text/plain)
2023-02-22 01:25 UTC, Andrew Bartlett 		jsutton: review+
Details
Patch v2 with CVE tag (13.23 KB, patch)
2023-02-26 21:40 UTC, Andrew Bartlett 		jsutton: review+
abartlet: ci-passed+
Details
Advisory v4 (2.14 KB, text/plain)
2023-03-13 22:15 UTC, Andrew Bartlett 		abartlet: review? (dbagnall)
jsutton: review+
Details
Patch v2 backported to Samba 4.18 (13.23 KB, patch)
2023-03-13 22:16 UTC, Andrew Bartlett 		jsutton: review+
abartlet: ci-passed+
Details
Patch v2 backported to Samba 4.17 (13.23 KB, patch)
2023-03-13 22:17 UTC, Andrew Bartlett 		no flags Details
Patch v2 for master backported to Samba 4.17 as v2.1 (16.54 KB, patch)
2023-03-14 21:27 UTC, Andrew Bartlett 		no flags Details
Patch v2 for master backported to Samba 4.17 as v2.1 (19.00 KB, patch)
2023-03-14 21:28 UTC, Andrew Bartlett 		abartlet: ci-passed+
Details
Patch v2 for master backported to Samba 4.17 as v2.2 (19.00 KB, patch)
2023-03-15 20:37 UTC, Andrew Bartlett 		abartlet: review? (dbagnall)
jsutton: review+
abartlet: ci-passed+
Details
Show Obsolete (8) 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 Ralph Böhme 2023-01-03 17:58:57 UTC 
From "Mitter, Lukas" <lmi@codemanufaktur.com> via security@samba.org:

To whom it may concern,

I am writing in regards to an unexpected behaviour on samba version 4.17.4.
We have done a routine check whether an unprivileged user can modify
attributes on objects.
The unprivileged user was able to delete the dNSHostName attribute via
"ldapmodify" and after further testing we found out that it was only
possible via a specific .ldif-File.

The following .ldif-File was able to delete the attribute via ldapmodify:

dn:
CN=PC0079-2,OU=Inactive,OU=Computers,OU=SecurityObjects,DC=ad,DC=codemanufaktur,DC=com
changetype: modify
delete: dNSHostName

The output of ldapmodify is as follows:
modifying entry
"CN=PC0079-2,OU=Inactive,OU=Computers,OU=SecurityObjects,DC=ad,DC=codemanufaktur,DC=com"


The similarly built .ldif-File which also should have the exact same
behaviour in the example selected, but was not able to delete the attribute
is the following:

dn:
CN=PC0079-2,OU=Inactive,OU=Computers,OU=SecurityObjects,DC=ad,DC=codemanufaktur,DC=com
changetype: modify
delete: dNSHostName
dNSHostName: PC0079-2.ad.codemanufaktur.com

The output of ldapmodify with this different .ldif-File is as follows:
modifying entry
"CN=PC0079-2,OU=Inactive,OU=Computers,OU=SecurityObjects,DC=ad,DC=codemanufaktur,DC=com"
ldap_modify: Insufficient access (50)
additional info: error in module acl: insufficient access rights during
LDB_MODIFY (50)

We expected the behaviour to be consistent on both instances and not
diverge.
We also expected that in both cases the unprivileged user is unable to
delete the attribute "dNSHostName".

We haven't found this inconsistent behaviour on any other attribute, it
only affects dNSHostName as it seems.
We have last attempted this exact same ldapmodify on the 1st July with
samba version 4.16.x installed and at that point in time the dNSHostName
Attribute could not be deleted.

Kind Regards
Comment 1 Douglas Bagnall 2023-01-03 21:39:52 UTC 
There was a bit of work in this area during 4.17, In particular CVE-2022-32743 
https://bugzilla.samba.org/show_bug.cgi?id=14833.
Comment 2 Douglas Bagnall 2023-01-04 08:45:05 UTC 
Created attachment 17718 [details]
test confirming the behaviour

The attachment adds tests that belong in acl.py, but acl.py is too big and unwieldy to iterate over easily, so really the answer is for acl.py to be split up.

Anyway, in a testenv, the unprivileged host name delete works if no host name is specified, whether by ldif, or an empty message (as you'd expect).

$ python3 /home/douglasb/src/samba-review/source4/dsdb/tests/python/acl_modify.py $SERVER -U"$USERNAME%$PASSWORD" --workgroup=$DOMAIN
time: 2023-01-04 08:39:51.755353Z
test: __main__.AclModifyTests.test_modify_delete_dns_host_name_ldif_specified
baseDN: DC=addom,DC=samba,DC=example,DC=com
time: 2023-01-04 08:39:51.867180Z
time: 2023-01-04 08:39:51.867225Z
successful: __main__.AclModifyTests.test_modify_delete_dns_host_name_ldif_specified
time: 2023-01-04 08:39:51.867361Z
test: __main__.AclModifyTests.test_modify_delete_dns_host_name_ldif_unspecified
baseDN: DC=addom,DC=samba,DC=example,DC=com
time: 2023-01-04 08:39:51.978212Z
time: 2023-01-04 08:39:51.978715Z
failure: __main__.AclModifyTests.test_modify_delete_dns_host_name_ldif_unspecified [
Traceback (most recent call last):
  File "/home/douglasb/src/samba-review/source4/dsdb/tests/python/acl_modify.py", line 228, in test_modify_delete_dns_host_name_ldif_unspecified
    self.assertRaisesLdbError(
  File "/home/douglasb/src/samba-review/bin/python/samba/tests/__init__.py", line 244, in assertRaisesLdbError
    self.fail("%s, expected "
AssertionError: User able to delete dNSHostName (without specific name), expected LdbError ERR_INSUFFICIENT_ACCESS_RIGHTS, (50) but we got success
]
time: 2023-01-04 08:39:51.978858Z
test: __main__.AclModifyTests.test_modify_delete_dns_host_name_specified
baseDN: DC=addom,DC=samba,DC=example,DC=com
time: 2023-01-04 08:39:52.089607Z
time: 2023-01-04 08:39:52.089666Z
successful: __main__.AclModifyTests.test_modify_delete_dns_host_name_specified
time: 2023-01-04 08:39:52.089808Z
test: __main__.AclModifyTests.test_modify_delete_dns_host_name_unspecified
baseDN: DC=addom,DC=samba,DC=example,DC=com
time: 2023-01-04 08:39:52.201214Z
time: 2023-01-04 08:39:52.201436Z
failure: __main__.AclModifyTests.test_modify_delete_dns_host_name_unspecified [
Traceback (most recent call last):
  File "/home/douglasb/src/samba-review/source4/dsdb/tests/python/acl_modify.py", line 197, in test_modify_delete_dns_host_name_unspecified
    self.assertRaisesLdbError(
  File "/home/douglasb/src/samba-review/bin/python/samba/tests/__init__.py", line 244, in assertRaisesLdbError
    self.fail("%s, expected "
AssertionError: User able to delete dNSHostName (without specified name), expected LdbError ERR_INSUFFICIENT_ACCESS_RIGHTS, (50) but we got success
]
time: 2023-01-04 08:39:52.201640Z
Comment 3 Douglas Bagnall 2023-01-04 21:34:18 UTC 
It looks like the problem is here, where it seems we forgot the delete case with no values:

https://gitlab.com/samba-team/devel/samba/-/blob/master/source4/dsdb/samdb/ldb_modules/acl.c#L949

and we need logic like "if it's a delete modify and not this or that condition, return insufficient rights error" inside the block. But I think I'll wait for Joseph to return because he will know.
Comment 4 Jo Sutton 2023-01-08 23:11:49 UTC 
Created attachment 17720 [details]
test and proposed fix for master

This bug was probably my fault. Thanks for spotting it.

The attached patch seems to fix the problem by moving the early return to where the dNSHostName value is actually used (which is only in the add or replace case), and having it return an error instead.
Comment 5 Jo Sutton 2023-01-09 01:01:46 UTC 
I suppose the CVSS score might be:
AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:L (5.4)

but I'm not sure to what degree availability would be affected, or whether an attacker could use this to gain extra privileges somehow.
Comment 6 Douglas Bagnall 2023-01-09 02:43:19 UTC 
(In reply to Joseph Sutton from comment #5)
Yep, it looks to be another contestant in the "non-admin user can be a real pain in the arse" category.

Do you have lines of thinking for gaining extra privileges? I am not getting very far, unless the attacker can also change dns records and/or put the dNSHostName somewhere else.

I guess we should get a CVE.

Also we might need to add tests around modify ADD and REPLACE in case there are regressions.
Comment 7 Jo Sutton 2023-01-09 03:46:22 UTC 
I couldn't think of anything, besides the possibility of a low-privileged user removing the dNSHostName from another account so as to add it to their own (with Write Property), thus masquerading as the other account; or of a user preventing other accounts from setting an SPN by removing their dNSHostNames.

We do have an existing test in acl.py, test_modify_dns_host_name_no_value, that does a REPLACE with no values and expects either success (as with Windows) or ERR_OPERATIONS_ERROR (as with Samba 4.17).
Comment 8 Jo Sutton 2023-01-11 03:11:15 UTC 
(In reply to Joseph Sutton from comment #7)
> We do have an existing test in acl.py, test_modify_dns_host_name_no_value, that
> does a REPLACE with no values and expects either success (as with Windows) or
> ERR_OPERATIONS_ERROR (as with Samba 4.17).
It's not testing exactly the same thing, however (the REPLACE is done under Validated Write privileges). I'll probably add another test to be sure.
Comment 9 Andrew Bartlett 2023-02-22 00:06:30 UTC 
(In reply to Joseph Sutton from comment #7)

Thankfully we don't do certificate authentication matching on the dnsHostName attribute, as that would be the best way to attack this.

But certainly a user could be quite a pain, we use this in DRS to find the other server, for example (thankfully with a fallback).  

So I score this as CVSS3.1:AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:L (5.4)
Comment 10 Andrew Bartlett 2023-02-22 00:24:40 UTC 
Lukas,

I am preparing our advisory now.

Can we credit you in our security advisory and do you have an affiliation you would like recorded?

Thanks!

Andrew Bartlett
Comment 11 Andrew Bartlett 2023-02-22 00:35:12 UTC 
Created attachment 17764 [details]
Initial advisory without versions

Advisory added, with credit.  I'll hide this one if the reporter doesn't want their name included.
Comment 12 Jo Sutton 2023-02-22 00:57:24 UTC 
Comment on attachment 17764 [details]
Initial advisory without versions

Looks good. Douglas should be credited for providing patches, as he wrote the tests for this.
Comment 13 Andrew Bartlett 2023-02-22 01:21:26 UTC 
Created attachment 17765 [details]
Updated advisory without versions (v2)

Updated to credit Douglas for the patches
Comment 14 Andrew Bartlett 2023-02-22 01:25:21 UTC 
Created attachment 17766 [details]
Advisory v3
Comment 15 Andrew Bartlett 2023-02-26 21:40:16 UTC 
Created attachment 17776 [details]
Patch v2 with CVE tag
Comment 16 Lukas Mitter 2023-02-27 10:04:02 UTC 
Andrew,

You may credit me.
With "affiliation" you are asking which company I work at?
In that case you may record it as "codemanufaktur GmbH".

Thank you!

Lukas Mitter
Comment 17 Andrew Bartlett 2023-03-13 22:15:20 UTC 
Created attachment 17812 [details]
Advisory v4

I've added the reporter's affiliation.
Comment 18 Andrew Bartlett 2023-03-13 22:16:25 UTC 
Created attachment 17813 [details]
Patch v2 backported to Samba 4.18
Comment 19 Andrew Bartlett 2023-03-13 22:17:25 UTC 
Created attachment 17814 [details]
Patch v2 backported to Samba 4.17
Comment 20 Andrew Bartlett 2023-03-14 21:27:03 UTC 
Created attachment 17817 [details]
Patch v2 for master backported to Samba 4.17 as v2.1

The patch for Samba 4.17 needed some extra prerequisite patches.
Comment 21 Andrew Bartlett 2023-03-14 21:28:38 UTC 
Created attachment 17818 [details]
Patch v2 for master backported to Samba 4.17 as v2.1
Comment 22 Andrew Bartlett 2023-03-15 20:37:07 UTC 
Created attachment 17833 [details]
Patch v2 for master backported to Samba 4.17 as v2.2

This updated version fixes the reversal of the first two dependent cherry-picked commits and has been compile tested on each commit, as well as being otherwise identical to the end product that was fully CI tested.
Comment 23 Andrew Bartlett 2023-03-16 00:23:32 UTC 
Assigning to Jule for next security release.
Comment 24 Andrew Bartlett 2023-03-19 22:44:20 UTC 
We will make a Samba security release for this issue on Wednesday 29 March
Comment 25 Jule Anger 2023-03-29 14:21:38 UTC 
Removing vendor CC (so that any public comments don't need to be broadcast so widely) and opening these bugs to the public.
If you wish to continue to be informed about any changes here please CC individually.
Comment 26 Samba QA Contact 2023-03-29 14:29:58 UTC 
This bug was referenced in samba v4-17-stable (Release samba-4.17.7):

b7af8aa2552e0690aac58fb98e3134b71f678ece
307b2e65d51903f6805460a2633ebe809d4052ab
54691236fc80a932f2069eef0aa21d6818445503
888c6ae8177d87e408722f67cc03359ae2533402
Comment 27 Samba QA Contact 2023-03-29 14:31:11 UTC 
This bug was referenced in samba v4-18-stable (Release samba-4.18.1):

016687b3aaea76abf4ae56523aa951547f806e38
003f6c16112a45af81ed59877d3b416a2f3847d9
Comment 28 Samba QA Contact 2023-03-29 14:36:36 UTC 
This bug was referenced in samba v4-17-test:

b7af8aa2552e0690aac58fb98e3134b71f678ece
307b2e65d51903f6805460a2633ebe809d4052ab
54691236fc80a932f2069eef0aa21d6818445503
888c6ae8177d87e408722f67cc03359ae2533402
Comment 29 Samba QA Contact 2023-03-29 14:40:35 UTC 
This bug was referenced in samba v4-18-test:

016687b3aaea76abf4ae56523aa951547f806e38
003f6c16112a45af81ed59877d3b416a2f3847d9
Comment 30 Samba QA Contact 2023-04-05 03:09:14 UTC 
This bug was referenced in samba master:

62cc4302b67d33a2fd57738cc9180f7b36d0cb9d
c33e78a27fbeb913b08ef7f74343c1f652d1aa41