Open m4rw3r opened 8 years ago
Much needed. This is actually a security issue. It would be good to at least document the current limitation.
For the time being, this is what I am using (no generics, as I don't think there's a trait for checked arithmetic):
pub fn decimal_u32<I: Input<Token = u8>>(i: I) -> SimpleResult<I, u32> {
take_while1(i, is_digit).bind(|i, it| {
let v = it.fold(Some(0u32), |x, c| {
x.and_then(|x| x.checked_mul(10))
.and_then(|x| x.checked_add((c - b'0') as _))
});
match v {
None => i.err(Error::unexpected()),
Some(acc) => i.ret(acc),
}
})
}
Problem
The integer-types are limited in what values they can represent, the
decimal
parser does not have an upper bound on the number of digits to parse or the maximum allowed value which can cause an overflow. It is also sometimes desirable to parse numbers in a certain range or with a specific number of digits.Combinators like
many
,many1
and similar also pose an issue as eg. a DoS vector since some patterns might cause the parser usingmany
(or similar) to allocate a lot of memory for theT: FromIterator
instance.Solution
To still make it easy to use the parsers and combinators the unbounded versions should still remain and be the default, but bounded versions need to be provided. Most of the issues (except for the overflow in
ascii::decimal
) can be avoided or limited by using abuffer::FixedSizeBuffer
or setting a limit onbuffer::GrowingBuffer
. Additional measures can be added on top of thebuffer::Source
or abuffer::Stream
instance.ascii::decimal
: can result in overflow since there is no limit to the number of digits it parsescombinators::many
:bounded::many
existscombinators::many1
:bounded::many
existscombinators::many_till
:bounded::many_till
existscombinators::sep_by
: essentiallybounded::many
but is lacking a bounded versioncombinators::sep_by1
: essentiallybounded::many
but is lacking a bounded versionbuffer::GrowingBuffer
: has a limit option