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

nocompressio is vulnerable to swapped chunks in state files #10463

Closed
zhaozhongn opened this issue May 17, 2024 · 2 comments · Fixed by #10914
Closed

nocompressio is vulnerable to swapped chunks in state files #10463

zhaozhongn opened this issue May 17, 2024 · 2 comments · Fixed by #10914
Assignees
Labels
type: bug Something isn't working

Comments

@zhaozhongn
Copy link
Contributor

Description

To prevent attack from swapping chunks in state files, the original compressio made each chunk's hash to be based on (data, size, previous_chunk_hash). The nocompressio is instead calculating chunk hash only based on (data, size). This means it will be vulnerable to swapped chunks in state files.

Steps to reproduce

No response

runsc version

No response

docker version (if using docker)

No response

uname

No response

kubectl (if using Kubernetes)

No response

repo state (if built from source)

No response

runsc debug logs (if available)

No response

@zhaozhongn zhaozhongn added the type: bug Something isn't working label May 17, 2024
@EtiennePerot
Copy link
Contributor

I'm not sure what type of threat this would protect against. If an attacker has the ability to modify the checkpoint file, it can also just corrupt it or replace any chunk with whatever it wants. The hash is only good for verifying integrity against random transcription errors; it does not protect against malicious rewriting.

That being said, I see no harm in making it robust against this regardless, so long as it can be done in a high-performance-friendly way (which was the intent of adding nocompressio mode in the first place). Adding in previous_chunk_hash to the computation forces the integrity verification to be serialized, because each expected hash requires checking the previous one. I suggest making the hash be over (data, size, index of chunk within checkpoint) instead.

Copy link

A friendly reminder that this issue had no activity for 120 days.

@github-actions github-actions bot added the stale-issue This issue has not been updated in 120 days. label Sep 15, 2024
@ayushr2 ayushr2 self-assigned this Sep 15, 2024
copybara-service bot pushed a commit that referenced this issue Sep 15, 2024
This is consistent with how compressio is implemented.

Fixes #10463

PiperOrigin-RevId: 674756781
@github-actions github-actions bot removed the stale-issue This issue has not been updated in 120 days. label Sep 16, 2024
copybara-service bot pushed a commit that referenced this issue Sep 17, 2024
This is consistent with how compressio is implemented.

Fixes #10463

PiperOrigin-RevId: 674756781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants