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

WSL-Helper: Certs: Make a copy of foreign memory #6308

Merged

Conversation

mook-as
Copy link
Contributor

@mook-as mook-as commented Jan 11, 2024

We enumerate system certificates on Windows asynchronously and return the results (as *x509.Certificate objects) in a channel. It turns out that those certificates can refer to memory passed in via ParseCertificate(), so we ended up using a certificate that referred to freed memory. Avoid the issue by explicitly making a copy of that slice.

Fixes #6295, fixes #6307

I'm basing this on release-1.12 in case we want to do a patch release; it rebases cleanly onto main if we don't want a patch release (just let me know).

We enumerate system certificates on Windows asynchronously and return the
results (as *x509.Certificate objects) in a channel.  It turns out that
those certificates can refer to memory passed in via ParseCertificate(),
so we ended up using a certificate that referred to freed memory.  Avoid
the issue by explicitly making a copy of that slice.

Signed-off-by: Mark Yen <mark.yen@suse.com>
Signed-off-by: Mark Yen <mark.yen@suse.com>
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

LGTM. I've also verified that the test fails without the copy and passes afterwards.

@jandubois jandubois merged commit ae3b6f2 into rancher-sandbox:release-1.12 Jan 11, 2024
10 checks passed
@mook-as mook-as deleted the win32/certs/copy-unsafe branch January 12, 2024 18:26
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.

None yet

2 participants