Protect encoding::decode_varint from overflow #527
Conversation
src/encoding.rs
Outdated
| @@ -167,7 +169,12 @@ where | |||
| let byte = buf.get_u8(); | |||
| value |= u64::from(byte & 0x7F) << (count * 7); | |||
| if byte <= 0x7F { | |||
| return Ok(value); | |||
| // check for u64::MAX overflow | |||
| if count == 9 && byte >= 2 { | |||
There was a problem hiding this comment.
Decided to add this check here in case moving it outside the first if caused some performance impact. This is the slow loop though...
ajguerrer
changed the title
|
Do you know if the C++ code protects against this? From what I can tell it does not. |
@LucioFranco, your right! The C++ code will happily parse and overflow. I found this while studying the Go code. I understand that fixing this bug has debatable usefulness. It will not hurt my feelings at all if you decide not to merge this. I just thought it was interesting. |
|
I appreciate it! @ajguerrer do you mind updating the code with a reference to that go code, both the function doc and the line you changed? I think if we have that then I am fine to merge, I just wanted to check if there was precedence. |
|
Oh and also rebase against master |
ajguerrer
force-pushed
the
decode-varint-overflow
branch
from
eef22be to
10c77b4
Compare
|
@LucioFranco hows this look? |
|
@ajguerrer there is one rustfmt error if you can fix that then we can merge |
|
@LucioFranco Once more! |
ajguerrer
force-pushed
the
decode-varint-overflow
branch
from
356ab54 to
0d6b3d7
Compare
Fixes the infamous #528.