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

Size issue in HeaderMap::reserve() #352

Closed
Qwaz opened this issue Nov 16, 2019 · 2 comments · Fixed by #360
Closed

Size issue in HeaderMap::reserve() #352

Qwaz opened this issue Nov 16, 2019 · 2 comments · Fixed by #360
Labels
A-headers Area: HTTP headers E-easy Effort: easy. Start here :D S-bug Severity: bug. Something is wrong!

Comments

@Qwaz
Copy link

Qwaz commented Nov 16, 2019

http/src/header/map.rs

Lines 622 to 640 in 9c05e39

pub fn reserve(&mut self, additional: usize) {
// TODO: This can't overflow if done properly... since the max # of
// elements is u16::MAX.
let cap = self.entries.len()
.checked_add(additional)
.expect("reserve overflow");
if cap > self.indices.len() {
let cap = cap.next_power_of_two();
if self.entries.len() == 0 {
self.mask = cap - 1;
self.indices = vec![Pos::none(); cap].into_boxed_slice();
self.entries = Vec::with_capacity(usable_capacity(cap));
} else {
self.grow(cap);
}
}
}

usize::next_power_of_two() method silently overflows to 0 in release mode. This makes it possible to shrink the size of the map to 0 with HeaderMap::reserve().

  • If the map doesn't contain any entry, it sets the mask value to usize::MAX which is inconsistent but doesn't create any immediate harm.
  • If the map contains any entry, the code will call self.grow(0) and start infinite probing in this line.

Another problem is that the assertion for MAX_SIZE doesn't exist here, so it is possible to grow the map larger than MAX_SIZE.

Demonstration

@hawkw
Copy link
Contributor

hawkw commented Nov 18, 2019

@Qwaz Am I correct that the potential vulnerability this presents is that an attacker could cause a denial of service by sending a request which causes this overflow?

@Qwaz
Copy link
Author

Qwaz commented Nov 19, 2019

@hawkw An attacker can trigger a DoS if they can call HeaderMap::reserve() to non-empty HeaderMap with a controlled parameter. I believe this scenario is unlikely in practice, but yes, it's theoretically possible.

@seanmonstar seanmonstar added A-headers Area: HTTP headers E-easy Effort: easy. Start here :D S-bug Severity: bug. Something is wrong! labels Nov 25, 2019
Qwaz added a commit to Qwaz/advisory-db that referenced this issue Jan 9, 2020
Qwaz added a commit to Qwaz/advisory-db that referenced this issue Jan 9, 2020
Qwaz added a commit to Qwaz/advisory-db that referenced this issue Jan 9, 2020
tarcieri added a commit to rustsec/advisory-db that referenced this issue Jan 9, 2020
roy-work added a commit to roy-work/advisory-db that referenced this issue Jan 9, 2020
…0.1.20

I believe these two vulnerabilities were patched at 0.1.20.

For RUSTSEC-2019-0033:

The advisory links to the bug: hyperium/http#352
In that bug, the fixing PR was hyperium/http#360
That PR merged the commit 81ceb61 to fix the bug; that commit, according to
GitHub, was first picked up by tag v0.1.20 ([commit][1]).

[1]: hyperium/http@81ceb61

For RUSTSEC-2019-0034:

This advisory is two separate GitHub issues against `HeaderMap::drain`,
http rustsec#354 and http rustsec#355.

For the first: the issue: hyperium/http#354
In that bug, the fixing PR was hyperium/http#357
That PR merged the commit 82d53db to fix the bug; that commit, according to
GitHub, was first picked up by tag v0.1.20 ([commit][2]).

[2]: hyperium/http@82d53db

For the second: the issue: hyperium/http#355
In that bug, the fixing PR was hyperium/http#362
That PR merged the commit 8ffe094 to fix the bug; that commit, according to
GitHub, was first picked up by tag v0.1.20 ([commit][3]).

[3]: hyperium/http@8ffe094
dfinity-bot added a commit to dfinity/sdk that referenced this issue Mar 5, 2020
Commits: [rustsec/advisory-db@891a872b...19196c29](rustsec/advisory-db@891a872...19196c2)

* [`6da6344b`](rustsec/advisory-db@6da6344) Add advisory for deprecated/unmaintained quickersort
* [`36b8de69`](rustsec/advisory-db@36b8de6) hyperium/http/issues/352
* [`ba2df66b`](rustsec/advisory-db@ba2df66) hyperium/http/issues/354,355
* [`0e59ecb7`](rustsec/advisory-db@0e59ecb) Assign RUSTSEC-2019-0033 to http
* [`526892a1`](rustsec/advisory-db@526892a) Assign RUSTSEC-2019-0034 to http
* [`200651cf`](rustsec/advisory-db@200651c) Correct affected version range on RUSTSEC-2019-003[34] to patched at 0.1.20
* [`57f553ee`](rustsec/advisory-db@57f553e) Add advisory for prost stack overflow
* [`7a0d254b`](rustsec/advisory-db@7a0d254) fixup! Add advisory for prost stack overflow
* [`a5b6099b`](rustsec/advisory-db@a5b6099) Assign RUSTSEC-2020-0002 to prost
* [`8b072513`](rustsec/advisory-db@8b07251) Fix typo
* [`e30a06a6`](rustsec/advisory-db@e30a06a) RUSTSEC-2016-0005: add note about rust-crypto vs RustCrypto
* [`17e82e13`](rustsec/advisory-db@17e82e1) Assign RUSTSEC-2018-0016 to quickersort
* [`b300fa84`](rustsec/advisory-db@b300fa8) Add unmaintained crate informational advisory: rust_sodium
* [`f8ff9cfc`](rustsec/advisory-db@f8ff9cf) Add lucet-runtime-internals sigstack allocation vuln advisory
* [`3f1f71de`](rustsec/advisory-db@3f1f71d) Update crates/lucet-runtime-internals/RUSTSEC-0000-0000.toml
* [`0271003e`](rustsec/advisory-db@0271003) Update crates/lucet-runtime-internals/RUSTSEC-0000-0000.toml
* [`2b82281e`](rustsec/advisory-db@2b82281) Assign RUSTSEC-2020-0003 (informational) to rust_sodium
* [`d8e872fd`](rustsec/advisory-db@d8e872f) Assign RUSTSEC-2020-0004 to lucet-runtime-internals
* [`df7657d3`](rustsec/advisory-db@df7657d) Fix broken/malformatted outbound links
* [`64c17acf`](rustsec/advisory-db@64c17ac) Migrate all advisories to V2 format (closes rustsec/advisory-db#228)
* [`38626513`](rustsec/advisory-db@3862651) .github: cache installation of rustsec-admin
* [`ce781096`](rustsec/advisory-db@ce78109) .github: fix rustsec-admin install caching
* [`f0ee46e9`](rustsec/advisory-db@f0ee46e) Migrate `rust/` advisories to V2 format
mergify bot added a commit to dfinity/sdk that referenced this issue Mar 9, 2020
Commits: [rustsec/advisory-db@891a872b...19196c29](rustsec/advisory-db@891a872...19196c2)

* [`6da6344b`](rustsec/advisory-db@6da6344) Add advisory for deprecated/unmaintained quickersort
* [`36b8de69`](rustsec/advisory-db@36b8de6) hyperium/http/issues/352
* [`ba2df66b`](rustsec/advisory-db@ba2df66) hyperium/http/issues/354,355
* [`0e59ecb7`](rustsec/advisory-db@0e59ecb) Assign RUSTSEC-2019-0033 to http
* [`526892a1`](rustsec/advisory-db@526892a) Assign RUSTSEC-2019-0034 to http
* [`200651cf`](rustsec/advisory-db@200651c) Correct affected version range on RUSTSEC-2019-003[34] to patched at 0.1.20
* [`57f553ee`](rustsec/advisory-db@57f553e) Add advisory for prost stack overflow
* [`7a0d254b`](rustsec/advisory-db@7a0d254) fixup! Add advisory for prost stack overflow
* [`a5b6099b`](rustsec/advisory-db@a5b6099) Assign RUSTSEC-2020-0002 to prost
* [`8b072513`](rustsec/advisory-db@8b07251) Fix typo
* [`e30a06a6`](rustsec/advisory-db@e30a06a) RUSTSEC-2016-0005: add note about rust-crypto vs RustCrypto
* [`17e82e13`](rustsec/advisory-db@17e82e1) Assign RUSTSEC-2018-0016 to quickersort
* [`b300fa84`](rustsec/advisory-db@b300fa8) Add unmaintained crate informational advisory: rust_sodium
* [`f8ff9cfc`](rustsec/advisory-db@f8ff9cf) Add lucet-runtime-internals sigstack allocation vuln advisory
* [`3f1f71de`](rustsec/advisory-db@3f1f71d) Update crates/lucet-runtime-internals/RUSTSEC-0000-0000.toml
* [`0271003e`](rustsec/advisory-db@0271003) Update crates/lucet-runtime-internals/RUSTSEC-0000-0000.toml
* [`2b82281e`](rustsec/advisory-db@2b82281) Assign RUSTSEC-2020-0003 (informational) to rust_sodium
* [`d8e872fd`](rustsec/advisory-db@d8e872f) Assign RUSTSEC-2020-0004 to lucet-runtime-internals
* [`df7657d3`](rustsec/advisory-db@df7657d) Fix broken/malformatted outbound links
* [`64c17acf`](rustsec/advisory-db@64c17ac) Migrate all advisories to V2 format (closes rustsec/advisory-db#228)
* [`38626513`](rustsec/advisory-db@3862651) .github: cache installation of rustsec-admin
* [`ce781096`](rustsec/advisory-db@ce78109) .github: fix rustsec-admin install caching
* [`f0ee46e9`](rustsec/advisory-db@f0ee46e) Migrate `rust/` advisories to V2 format

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-headers Area: HTTP headers E-easy Effort: easy. Start here :D S-bug Severity: bug. Something is wrong!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants