Category: Coding Mistakes
Bad decimal-parsing function accepts multiple dots
Low Severity
Low Impact
Low Likelihood
Description
The decimal-parsing function shown below is incorrectly implemented. A variant of this function is used in all targets listed.
public fun from_string(num: &String): Decimal256 {
let vec = string::bytes(num);
let len = vector::length(vec);
let cursor = 0;
let dot_index = 0;
let val: u256 = 0;
while (cursor < len) {
let s = *vector::borrow(vec, cursor);
cursor = cursor + 1;
// find `.` position
if (s == 46) continue;
val = val * 10;
assert!(s >= 48 && s <= 57, error::invalid_argument(EFAILED_TO_DESERIALIZE));
let n = (s - 48 as u256);
val = val + n;
if (cursor == dot_index + 1) {
// use `<` not `<=` to safely check "out of range"
// (i.e. to avoid fractional part checking)
assert!(val < MAX_INTEGER_PART, error::invalid_argument(EOUT_OF_RANGE));
dot_index = dot_index + 1;
};
};
// ignore fractional part longer than `FRACTIONAL_LENGTH`
let val = if (dot_index == len) {
val * pow(10, FRACTIONAL_LENGTH)
} else {
let fractional_length = len - dot_index - 1;
if (fractional_length > FRACTIONAL_LENGTH) {
val / pow(10, fractional_length - FRACTIONAL_LENGTH)
} else {
val * pow(10, FRACTIONAL_LENGTH - fractional_length)
}
};
new(val)
}If there is more than one dot, it will still parse the value, although it will return a corrupted result.
Impact
Since output from this function from the user should be checked anyway, we have labelled this as low impact. Any attacker trying to take advantage of this bug could instead simply enter the correct formatting of the number that would be returned by an incorrect parsing of this function.
Recommendations
Adjust the algorithm to handle inputs with multiple dots in a more intuitive manner.