Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows/certificates: Fix bug in environment variable expansion #5697

Merged
merged 4 commits into from
Aug 13, 2019

Conversation

offlinemark
Copy link
Contributor

@offlinemark offlinemark commented Aug 8, 2019

This is a followup to #5640. This should be the last one :)

This fixes a bug in environment variable expansion that occurs when paths to Personal certificate directories are constructed for system accounts.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 8, 2019

CLA Check
The committers are authorized under a signed CLA.

@offlinemark
Copy link
Contributor Author

Maybe @muffins or @theopolis could look? Sorry for all the follow-ons 😅 This should be the last one.

Copy link
Member

@alessandrogario alessandrogario left a comment

Choose a reason for hiding this comment

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

I observed rare cases where it would just write 1 wchar_t and return.
Add a loop that repeatedly retries up to 100 times before returning
failure to the caller.
@offlinemark offlinemark force-pushed the mark/fix/win-certs-env-expand branch from 5ae7cdd to 24f6032 Compare August 9, 2019 00:03
@offlinemark offlinemark changed the title windows/certificates: Correctly handle incomplete writes from ExpandEnvironmentStringsW windows/certificates: Fix bug in environment variable expansion Aug 9, 2019
@theopolis theopolis merged commit c3b3476 into osquery:master Aug 13, 2019
@offlinemark offlinemark deleted the mark/fix/win-certs-env-expand branch August 14, 2019 20:53
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.

4 participants