security: validate QCOW2 ClusterBits to prevent OOM from crafted headers#1995
Open
adilburaksen wants to merge 1 commit intogoogle:mainfrom
Open
security: validate QCOW2 ClusterBits to prevent OOM from crafted headers#1995adilburaksen wants to merge 1 commit intogoogle:mainfrom
adilburaksen wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Author
|
Updated with two additional fixes per code review:
All 38 tests pass. |
Three header fields are used as allocation sizes without validation: 1. ClusterBits — used as shift exponent: 1<<ClusterBits. Value 33 → 8 GB make([]byte, 8GB) in readL2Table (format.go:218). Confirmed live: 116-byte file causes "runtime: out of memory". 2. L1Size — used directly: make([]uint64, L1Size). Value 0x1FFFFFFF → 4 GB allocation in readL1Table. 3. RefcountTableClusters — multiplied with uint32(clusterSize) in readRefcountTable. With valid ClusterBits=9, value 0x7FFFFF yields a ~4 GB uint32 result without overflow. Fix: reject all three out-of-range values in parseHeader before any clusterSize or allocation is computed. - ClusterBits: spec range [9, 21] - L1Size: cap at 2M entries (covers ≥64 TiB at minimum cluster size) - RefcountTableClusters: cap at 64 (typical images use 1–3) Regression test: TestConvertQCOW2ClusterBitsRejected confirms 116-byte malicious input is rejected with 0 MB TotalAlloc delta. All 38 existing tests pass.
36260d7 to
c3a62c2
Compare
Author
|
Hi! Just a friendly ping — all CI checks are passing. Happy to address any review feedback whenever you get a chance. Thanks for your time! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
parseHeaderinextractor/filesystem/embeddedfs/qcow2/format.godoes not validate theClusterBitsfield from the QCOW2 header. Every code path that computesclusterSize = uint64(1) << header.ClusterBitsthen passes that value directly tomake()orbytes.Repeat()without bounds checking.Impact: A 116-byte crafted
.qcow2file withClusterBits=33causes scalibr to attempt an 8 GB heap allocation insidereadL2Table, crashing the process withruntime: out of memory. Values up to 62 produce allocations up to 4 EB.Root Cause
Same pattern at:
readRefcountBlock:316,readCluster:232,writeRawImage:340.Fix
Validate
ClusterBitsagainst the QCOW2 spec (9–21) inparseHeader, before anyclusterSizeis computed:Test
TestConvertQCOW2ClusterBitsRejectedinsecurity_regression_test.go:runtime: out of memoryatreadL2Table:format.go:218All existing qcow2 tests pass (18/18).