Provide alternatives preventing overflow and unbounded parsing
Opened this issue · 2 comments
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 using many
(or similar) to allocate a lot of memory for the T: 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 a buffer::FixedSizeBuffer
or setting a limit on buffer::GrowingBuffer
. Additional measures can be added on top of the buffer::Source
or a buffer::Stream
instance.
-
ascii::decimal
: can result in overflow since there is no limit to the number of digits it parses -
combinators::many
:bounded::many
exists -
combinators::many1
:bounded::many
exists -
combinators::many_till
:bounded::many_till
exists -
combinators::sep_by
: essentiallybounded::many
but is lacking a bounded version -
combinators::sep_by1
: essentiallybounded::many
but is lacking a bounded version -
buffer::GrowingBuffer
: has a limit option
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),
}
})
}