Skip to content

Commit fe2be6f

Browse files
senamakelmedullabot
andcommitted
fix: deny null DACL before native ACL accessor
Co-authored-by: Medulla <medulla@tinyhumans.ai>
1 parent f037896 commit fe2be6f

4 files changed

Lines changed: 45 additions & 6 deletions

File tree

‎crates/tinysecurity-audit/src/sink_windows_negative_tests.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -356,14 +356,14 @@ exit 0
356356
# localized Win32-message suffix against this bounded numeric vocabulary.
357357
# Raw exception text stays in memory and ambiguous matches report zero.
358358
$message = $_.Exception.Message.TrimEnd([char[]]@('.', ' ', [char]13, [char]10))
359-
$matches = @()
359+
$nativeMatches = @()
360360
foreach ($candidate in @(2, 3, 5, 8, 14, 87, 193, 233, 267, 740, 1058, 1060, 1314, 1326, 1330, 1385, 1450, 1783)) {
361361
$suffix = [ComponentModel.Win32Exception]::new($candidate).Message.TrimEnd([char[]]@('.', ' ', [char]13, [char]10))
362-
if ($message.EndsWith($suffix, [StringComparison]::Ordinal)) { $matches += $candidate }
362+
if ($message.EndsWith($suffix, [StringComparison]::Ordinal)) { $nativeMatches += $candidate }
363363
}
364364
$derived = 0
365-
if ($matches.Count -eq 1) { $derived = $matches[0] }
366-
Write-Output ('audit-account-stage=' + $stage + ' message-derived-code=' + $derived + ' candidates=' + $matches.Count)
365+
if ($nativeMatches.Count -eq 1) { $derived = $nativeMatches[0] }
366+
Write-Output ('audit-account-stage=' + $stage + ' message-derived-code=' + $derived + ' candidates=' + $nativeMatches.Count)
367367
$exception = $_.Exception
368368
for ($depth = 0; ($null -ne $exception) -and ($depth -lt 8); $depth++) {
369369
$native = 0

‎crates/tinysecurity-audit/src/windows.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,12 +213,21 @@ fn valid_name(path: &Path) -> Result<()> {
213213
Ok(())
214214
}
215215
fn descriptor(file: &File) -> Result<LocalBox<SecurityDescriptor>> {
216-
wrappers::GetSecurityInfo(
216+
let sd = wrappers::GetSecurityInfo(
217217
file,
218218
SeObjectType::SE_FILE_OBJECT,
219219
SecurityInformation::Owner | SecurityInformation::Dacl,
220220
)
221-
.map_err(|_| Error::Permissions)
221+
.map_err(|_| Error::Permissions)?;
222+
let sddl = sd
223+
.as_sddl()
224+
.map_err(|_| Error::Permissions)?
225+
.into_string()
226+
.map_err(|_| Error::Permissions)?;
227+
if !crate::windows_acl::has_dacl_entries(&sddl) {
228+
return Err(Error::Permissions);
229+
}
230+
Ok(sd)
222231
}
223232
fn sid_text(sid: &Sid) -> Result<String> {
224233
wrappers::ConvertSidToStringSid(sid)

‎crates/tinysecurity-audit/src/windows_acl.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ pub(crate) fn protected(mut flags: &str) -> bool {
2222
}
2323
seen & 1 != 0
2424
}
25+
// A present NULL DACL permits everyone, but the safe dependency's native
26+
// dacl() accessor panics for it. Check the canonical native SDDL first. This
27+
// does not validate ACE rights: the existing handle checks still do that.
28+
pub(crate) fn has_dacl_entries(sddl: &str) -> bool {
29+
sddl.split("D:")
30+
.nth(1)
31+
.and_then(|dacl| dacl.split("S:").next())
32+
.is_some_and(|dacl| dacl.contains('('))
33+
}
2534
#[cfg(test)]
2635
#[path = "windows_acl_tests.rs"]
2736
mod tests;

‎crates/tinysecurity-audit/src/windows_acl_tests.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,24 @@ fn windows_unprotected_unknown_or_duplicate_dacl_control_tokens_are_denied() {
1515
assert!(!protected(flags), "unacceptable control flags: {flags}");
1616
}
1717
}
18+
19+
#[test]
20+
fn windows_null_absent_and_empty_dacls_are_rejected_before_native_acl_access() {
21+
for sddl in [
22+
"O:SY",
23+
"O:SYD:",
24+
"O:SYD:P",
25+
"O:SYD:NO_ACCESS_CONTROL",
26+
"O:SYD:PNO_ACCESS_CONTROL",
27+
"O:SYS:(AU;SA;FA;;;WD)",
28+
"O:SYD:PS:(AU;SA;FA;;;WD)",
29+
] {
30+
assert!(!has_dacl_entries(sddl));
31+
}
32+
for sddl in [
33+
"O:SYD:P(A;OICI;FA;;;SY)",
34+
"O:SYD:(A;;FR;;;WD)S:(AU;SA;FA;;;WD)",
35+
] {
36+
assert!(has_dacl_entries(sddl));
37+
}
38+
}

0 commit comments

Comments
 (0)