Skip to content

[V-PHX-VUL-022] Incorrect decimals assertion #184

Closed
@gangov

Description

@gangov

The function from_str in the DECIMAL crate is used to convert a string to a decimal type. To handle the fractional part of the number, the function has the following logic:

Code snippet from the from_str function. It handles the fractional part of a string number.

if let Some(fractional_part) = parts_iter.next() {
   let fractional: i128 = fractional_part.parse().expect("Error parsing fractional");
   let exp = Self::DECIMAL_PLACES - fractional_part.len() as i32;
   assert!(exp <= Self::DECIMAL_PLACES, "Too many fractional digits");
   let fractional_factor = 10i128.pow(exp as u32);
   atomics += fractional * fractional_factor;
}

As it can be noted, the assert! is a tautology since exp will always be greater or equal than DECIMAL_PLACES given that fractional_part.len() is a positive number. Given the this, then it is possible for a negative exp to pass the assert! which later will be casted to a u32 number.

Impact

At present, there is no immediate consequence to this situation. The limitation stems from the fact that the largest string permissible as input can only consist of 36 digits for its fractional part. Should this limit be exceeded, invoking fractional_part.parse() would result in a panic. Conversely, the smallest accepted value for exp is -18, which, upon being casted to u32, transforms into a substantial number making the next line with pow to panic due to an overflow.

Recommendation

The assert should be changed to assert!(exp >= 0).

Metadata

Metadata

Assignees

Labels

auditbugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions