Skip to content

Handle 0-length domain names in Advapi32Util#getAccountBySid#1322

Merged
dbwiddis merged 2 commits intojava-native-access:masterfrom
dbwiddis:accountbysid
Mar 6, 2021
Merged

Handle 0-length domain names in Advapi32Util#getAccountBySid#1322
dbwiddis merged 2 commits intojava-native-access:masterfrom
dbwiddis:accountbysid

Conversation

@dbwiddis
Copy link
Copy Markdown
Contributor

@dbwiddis dbwiddis commented Mar 5, 2021

In rare/transient situations, such as UAC elevation or other SYSTEM account situations, the domain name can be an empty string. In this case cchDomainName is set to 0 and a zero-length array causes the function call to fail with an incorrect parameter. See oshi/oshi#1551 and oshi/oshi#637 for examples of failure symptoms.

Rather than adding code to handle the special case, we can simplify the code to avoid the initial call to get length, and prevent this error by using the max possible string lengths.

GetUserName docs indicate:

A buffer size of (UNLEN + 1) characters will hold the maximum length user name including the terminating null character. UNLEN is defined in Lmcons.h.

UNLEN is 256. The max local domain length (DNLEN) is 15, but to be conservative I used the max FQDN length of 255.

@dbwiddis
Copy link
Copy Markdown
Contributor Author

dbwiddis commented Mar 5, 2021

This sample code from MSDN does a single call using MAX_NAME of 256 for both fields (which apparently works without null terminator because it's a fixed length array). One potential needed change is to test for the zero-length domain name when building the fqn field. It's not clear from the docs that the length parameter is reset when the function doesn't fail.

@dbwiddis dbwiddis force-pushed the accountbysid branch 2 times, most recently from 4b7f79a to c846543 Compare March 5, 2021 20:02
Copy link
Copy Markdown
Member

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@dbwiddis dbwiddis merged commit 25194e1 into java-native-access:master Mar 6, 2021
@dbwiddis dbwiddis deleted the accountbysid branch March 6, 2021 20:16
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.

2 participants