Skip to content

[platform-win] Add null check to pOldDACL#4735

Open
xmkg wants to merge 2 commits intomainfrom
bugfix/win-add-missing-dacl-null-check
Open

[platform-win] Add null check to pOldDACL#4735
xmkg wants to merge 2 commits intomainfrom
bugfix/win-add-missing-dacl-null-check

Conversation

@xmkg
Copy link
Member

@xmkg xmkg commented Mar 16, 2026

The new_ACL function assumes that pOldDACL ptr would be non-empty after a successful GetNamedSecurityInfo call, but that premise does not exist

Excerpt from GetNamedSecurityInfo docs:

A pointer to a variable that receives a pointer to the DACL in the
returned security descriptor or NULL if the security descriptor has
no DACL. The returned pointer is valid only if you set the DACL_SECURITY_INFORMATION
flag. Also, this parameter can be NULL if you do not need the DACL.

NULL DACL is a valid return value, indicating that the queried security info has no DACL (i.e., no access restrictions).

The patch adds a null check to fix that.

new_ACL function assumes that pOldDACL ptr would be non-empty after a
successful `GetNamedSecurityInfo` call but that premise does not exists

Excerpt from `GetNamedSecurityInfo` docs:

> A pointer to a variable that receives a pointer to the DACL in the
> returned security descriptor or NULL if the security descriptor has
> no DACL. The returned pointer is valid only if you set the DACL_SECURITY_INFORMATION
> flag. Also, this parameter can be NULL if you do not need the DACL.

NULL DACL is a valid return value which means the queried security info
has no DACL info (i.e., no access restrictions).

The patch adds a null check to fix that.

Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
@xmkg xmkg requested a review from tobe2098 March 16, 2026 17:02
Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.65%. Comparing base (dd0b47c) to head (ef1e7c5).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4735   +/-   ##
=======================================
  Coverage   87.65%   87.65%           
=======================================
  Files         258      258           
  Lines       14124    14124           
=======================================
  Hits        12379    12379           
  Misses       1745     1745           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant