Skip to content

BUGFIX: don't allocate more memory than what can be read from io.Reader#42

Merged
johnweldon merged 1 commit intogo-asn1-ber:masterfrom
inteon:fix_memory_issues
Apr 22, 2024
Merged

BUGFIX: don't allocate more memory than what can be read from io.Reader#42
johnweldon merged 1 commit intogo-asn1-ber:masterfrom
inteon:fix_memory_issues

Conversation

@inteon
Copy link
Copy Markdown
Contributor

@inteon inteon commented Apr 10, 2024

Memory was allocated based on encoded length instead of the length of the io.Reader.
This makes it possible for an attacker to cause DOS due to unexpected high memory usage (max MaxPacketLengthBytes) with a reader is actually limited in length.

Comment thread fuzz_test.go
// make([]byte, length) can allocate up to MaxPacketLengthBytes which is
// currently 2 GB. This can cause memory related crashes when fuzzing in
// parallel or on memory constrained devices.
MaxPacketLengthBytes = 65536
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes the issue described above.

…ength of the io.Reader, causing potential DOS due to unexpected high memory usage (max MaxPacketLengthBytes)

Signed-off-by: Tim Ramlot <[email protected]>
@inteon inteon force-pushed the fix_memory_issues branch from 6f0d3c6 to e0b83e0 Compare April 12, 2024 14:32
@inteon
Copy link
Copy Markdown
Contributor Author

inteon commented Apr 12, 2024

I replaced io.ReadAll with ioutil.ReadAll, so the code works on go <= 1.15 too.

@inteon
Copy link
Copy Markdown
Contributor Author

inteon commented Apr 22, 2024

@johnweldon PTAL

Copy link
Copy Markdown
Contributor

@johnweldon johnweldon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you

@johnweldon johnweldon merged commit 5679dfd into go-asn1-ber:master Apr 22, 2024
@inteon
Copy link
Copy Markdown
Contributor Author

inteon commented Apr 22, 2024

@johnweldon Thanks for merging the PR, can this be a 1.5.6 release?

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