Change download file name to match the installer URL #1722
Conversation
| std::filesystem::path filename = GetFileNameFromURI(context.Get<Execution::Data::Installer>()->Url); | ||
|
|
||
| // Assuming that we find a stem value in the URI, use it | ||
| if (filename.has_stem()) |
There was a problem hiding this comment.
Are you concerned about the "it already has an executable extension" or the "that is a weird file name" or the "we will create an even weirder file name"?
I don't think we care about weird file names; this is more in line with the name that the developer created (although maybe not perfectly).
As to the file having an executable extension before being hashed, you could already do that by setting PackageVersion: exe. If we want to truly avoid that then we will have to change the code a bit more to use the hash as the temp file name. But then the "rename with extension" code will have to generate the whole name. In any case I see it as defense in depth of the general case, not a true security barrier to a dedicated attacker. But if you feel strongly we can discuss it more.
There was a problem hiding this comment.
I don't feel strongly about the concern. It just does not match what the comment says at the top of the method. So just wanted to confirm we are ok with the less common cases.
|
Thanks for fixing! Looks good to me. |
Change
Change the installer download file from:
to
Example:
becomes
In addition, this change uses:
as the name of the file before hash validation. This prevents the potential for any user input to create an executable file name since the SHA256 hash must be a hexadecimal string.
Validation
Existing regression tests and manual runs to ensure consistency for golden path.
Microsoft Reviewers: Open in CodeFlow