Bug 15609 - Conditional ACE SDDL round-trip failure
Summary: Conditional ACE SDDL round-trip failure
Status: NEW
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Samba QA Contact
QA Contact: Samba QA Contact
URL: https://bugs.chromium.org/p/oss-fuzz/...
Depends on:
Reported: 2024-03-19 21:30 UTC by Douglas Bagnall
Modified: 2024-03-19 21:30 UTC (History)
0 users

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Douglas Bagnall 2024-03-19 21:30:59 UTC
In https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65449 OSS-Fuzz has found an SDDL->SD->SDDL->SD round-trip failure.

The string  "S:(XA;;;;;KA;((!@USER.E)>4))" is being round tripped as "S:(XA;;;;;KA;(!(@USER.E) > 4))", turning "((!@USER.E) > 4)" into "(!(@USER.E) > 4)", which then parses as a different security descriptor.

I think the real bug is in the *serialisation*, not the parsing, though the parser has issues here too.

The original SDDL "((!@USER.E) > 4)" is not quite valid, but our parser can't see that because it doesn't look far ahead. By the rules, "!" can only appear before a parenthetical phrase return a boolean result type, or before a "@"-attribute (as in "@USER.E"). A boolean result can't be compared to a literal integer (except to 0 and 1, with == or !=). So this conditional ACE will fail when run, because the ">" operator will baulk (except it will never be run because it is in the SACL).

The reserialised form "(!(@USER.E) > 4)" is correctly parsed as equivalent to "(!(@USER.E > 4))".

To summarise:

1. The invalid ACE produced by "((!@USER.E) > 4)" could easily be made other ways, so papering over the SDDL parsing bug is not really solving much.

2. The invalid ACE is not handled wrongly in itself.

3. The wrong serialisation of the invalid ACE is a bug, but not a security issue.

Perhaps we should always try to run a conditional ACE with a dummy security token before serialising it.