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

Add and Validate NestedInstaller FileSha256 #2675

Closed
wants to merge 4 commits into from

Conversation

Trenly
Copy link
Contributor

@Trenly Trenly commented Nov 7, 2022

Still working on adding unit and E2E tests

Microsoft Reviewers: Open in CodeFlow

@Trenly Trenly changed the title Base implementation of NestedInstaller FileSha256 Add and Validate NestedInstaller FileSha256 Nov 7, 2022
@yao-msft
Copy link
Contributor

yao-msft commented Nov 9, 2022

Just my 2 cents before we get too far into implementation. I'm not sure if this is of high importance since we already validated the zip hash before extracting. If we are concerned that extracted files may be tampered after extraction and before execution, we could improve the code to put a write exclusive handle on to be extracted files and keep the handle after execution completes.

@Trenly
Copy link
Contributor Author

Trenly commented Nov 9, 2022

Just my 2 cents before we get too far into implementation. I'm not sure if this is of high importance since we already validated the zip hash before extracting. If we are concerned that extracted files may be tampered after extraction and before execution, we could improve the code to put a write exclusive handle on to be extracted files and keep the handle after execution completes.

I think that is certainly one concern, but the other concern I have is that it is much easier to cause a hash collision on Zip files than other file types. Presume for a moment that someone had a malicious application, put it inside a zip, and included in the zip a second binary file that had been carefully crafted to make the hash of the overall zip file the same. That would pass hash validation still. Ignoring the fact that malware scans would probably catch it anyways, its just an optional added layer of protection to validate the hash of the file that was actually extracted

@Trenly Trenly closed this Jan 31, 2023
@Trenly Trenly deleted the NestedInstallerHash branch January 31, 2023 14:29
@Trenly
Copy link
Contributor Author

Trenly commented Jan 31, 2023

Closed as this is not a high-priority item, I don't feel like resolving merge conflicts, and there are probably better implementations that could be planned for the future

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