Skip to content

embeddedfs/vdi: add bounds check for BlocksInImage before slice allocation#1994

Open
adilburaksen wants to merge 2 commits intogoogle:mainfrom
adilburaksen:fix/vdi-blocksinimage-bounds-check
Open

embeddedfs/vdi: add bounds check for BlocksInImage before slice allocation#1994
adilburaksen wants to merge 2 commits intogoogle:mainfrom
adilburaksen:fix/vdi-blocksinimage-bounds-check

Conversation

@adilburaksen
Copy link
Copy Markdown

Summary

convertVDIToRaw calls make([]uint32, hdr.BlocksInImage) with no bounds
check on the untrusted BlocksInImage header field (vdi.go:197).

BlocksInImage is a uint32 read directly from the file. Setting it to
0x3FFFFFFF requests a ~4 GB slice; 0xFFFFFFFF requests ~16 GB. The
allocation occurs before any I/O — a 512-byte file is sufficient to trigger it.

Before fix (512-byte input):

TotalAlloc delta: 8192 MB

On systems with overcommit disabled (common in containers with
vm.overcommit_memory=2), the make() call panics the process immediately.

Fix

Add a hard cap of 64 << 20 blocks (67,108,864) before the allocation.
This covers disks ≥ 64 TiB at the minimum 1 MiB VDI block size — far beyond
any realistic disk image a scanner would encounter.

const maxBlocksInImage = 64 << 20
if hdr.BlocksInImage > maxBlocksInImage {
    return fmt.Errorf("BlocksInImage %d exceeds safety limit %d", ...)
}
indices := make([]uint32, hdr.BlocksInImage)

Testing

TestConvertVDIOOMRejected constructs a minimal 512-byte VDI header with
BlocksInImage=0x3FFFFFFF in pure Go and confirms the fixed code returns
an error with zero heap allocation.

After fix: TotalAlloc delta: 0 MB, immediate error.

…ation

convertVDIToRaw called make([]uint32, hdr.BlocksInImage) with no bounds
check on the untrusted header field. A 512-byte crafted VDI with
BlocksInImage=0x3FFFFFFF triggers a ~4 GB heap allocation before any
I/O is attempted. With BlocksInImage=0xFFFFFFFF the request reaches ~16 GB.

Fix: reject BlocksInImage values above 64M blocks (covers disks ≥64 TiB
at the minimum 1 MiB block size) before the allocation.

Add TestConvertVDIOOMRejected to confirm the malicious header is rejected.
@adilburaksen
Copy link
Copy Markdown
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!

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.

1 participant