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

Refactor: Path.GetTempFileName() is considered insecure in certain scenarios #23037

Closed
wants to merge 1 commit into from

Conversation

guilhermelinosp
Copy link

Description of Change

Replaced Path.GetTempFileName() with Path.GetRandomFileName() to prevent insecure temporary file creation, mitigating race condition issues and enhancing security.

Use Path.GetRandomFileName() and manually create the file securely, ensuring atomic creation with non-predictable names. This mitigates the risk of race conditions and unauthorized access.

Issues Fixed

Copy link
Contributor

Hey there @guilhermelinosp! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jun 13, 2024
@MartyIX
Copy link
Contributor

MartyIX commented Jun 13, 2024

Why are you creating so many PRs and closing them immediately? :)

@guilhermelinosp
Copy link
Author

I changed the branch name and the PR closed, then I opened it again

@rmarinho rmarinho requested a review from mattleibow June 13, 2024 16:46
@PureWeen PureWeen added the area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing label Jun 20, 2024
_tempFilePath = Path.GetTempFileName();
_tempFilePath = Path.GetRandomFileName();
Copy link
Member

Choose a reason for hiding this comment

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

Can you share a link discussing GetTempFileName()? The one I found was:

I actually don't think this code is correct, as GetTempFileName() returns a full path and you have to use Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()) otherwise.

Did you also check the entire codebase? I find it hard to believe this was used in one place.

Copy link
Member

Choose a reason for hiding this comment

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

If we really want to "ban" GetTempFileName() we could also list it here:

And anyone would get a build error if they used it in a non-test project.

@mattleibow
Copy link
Member

Can you also open an issue in the dotnet runtime repo? Rather that working around the few case maui has, the whole dotnet system should either fix or avoid it.

@PureWeen
Copy link
Member

Closing this for the time being. Please raise an issue/discussion addressing @jonathanpeppers questions and then we can get a PR reviewed and merged

@PureWeen PureWeen closed this Jun 20, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants