Skip to content

Commit

Permalink
Fix underflow in modexp gas calculation (#883)
Browse files Browse the repository at this point in the history
## Description

In `calc_iter_count()`, which is part of the modexp precompile gas
calculation, the most-significant bit of the exponent is added to the
intermediate result. The MSB is computed using `U256::from(exp.bits()) -
U256::from(1))` in:

```rust
Ok(U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1))
```

This works only if the exponent is non-zero; subtracting `1` from zero
results in an underflow, leading to inaccurate gas requirements for
specific inputs.

Geth computes the MSB [as
follows](https://github.com/ethereum/go-ethereum/blob/fa0df76f3cfd186a1f06f2b80aa5dbb89555b009/core/vm/contracts.go#L340);
notice the condition `bitlen > 0`:

```go
	var msb int
	if bitlen := expHead.BitLen(); bitlen > 0 {
		msb = bitlen - 1
	}
```

This PR changes the underflowing subtraction into a `saturating_sub` so
that an underflow never occurs.

## Performance / NEAR gas cost considerations

## Testing

Fuzzing.

## How should this be reviewed

A test has been added to `modexp.rs` which asserts that the required gas
for the specified input is `65536`; an equivalent Go program which uses
Geth to demonstrate this is provided below:

```go
package main

import (
    "github.com/ethereum/go-ethereum/core/vm"
    "github.com/ethereum/go-ethereum/common"
    "fmt"
)

func main() {
    input := []byte{
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, 0x00, 0x26, 0x00}
    modexp_address := common.BytesToAddress([]byte{0x05})
    gas := vm.PrecompiledContractsBerlin[modexp_address].RequiredGas(input)
    fmt.Println(gas)
}
```

## Additional information

Co-authored-by: Oleksandr Anyshchenko <[email protected]>
  • Loading branch information
guidovranken and aleksuss authored Dec 11, 2023
1 parent e05b6b7 commit c7771cb
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion engine-precompiles/src/modexp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<HF: HardFork, M: ModExpAlgorithm> ModExp<HF, M> {
Ok(U256::from(exp.bits()) - U256::from(1))
} else {
// else > 32
Ok(U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1))
Ok(U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits().saturating_sub(1)))
}
}

Expand Down Expand Up @@ -726,4 +726,12 @@ mod tests {
ModExp::<Berlin>::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false);
assert!(res.is_ok());
}

#[test]
fn test_calc_iter_count_underflow() {
let input = hex::decode("0000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000002600");
let gas = ModExp::<Berlin>::required_gas(&input.unwrap()).unwrap();
let min_gas = EthGas::new(65536);
assert_eq!(gas, min_gas);
}
}

0 comments on commit c7771cb

Please sign in to comment.