Commit 5221af89 authored by smartcontracts's avatar smartcontracts Committed by Adrian Sutton

fix: have MIPS revert on add/sub overflow/underflow (#230)

Updates MIPSInstructions so that it correctly reverts on calls to
add, addi, and sub that overflow/underflow. Additionally includes
tests that demonstrates that the unchecked versions of the same
opcodes allow for overflow/underflow.
parent 5bd46ac5
......@@ -186,11 +186,29 @@ func ExecuteMipsInstruction(insn, opcode, fun, rs, rt, mem uint32) uint32 {
return rs
// The rest includes transformed R-type arith imm instructions
case 0x20: // add
return rs + rt
intRs := int32(rs)
intRt := int32(rt)
intRes := intRs + intRt
if intRs > 0 && intRt > 0 && intRes <= 0 {
panic("MIPS: add overflow")
}
if intRs < 0 && intRt < 0 && intRes >= 0 {
panic("MIPS: add underflow")
}
return uint32(intRes)
case 0x21: // addu
return rs + rt
case 0x22: // sub
return rs - rt
intRs := int32(rs)
intRt := int32(rt)
intRes := intRs - intRt
if intRs > 0 && intRt < 0 && intRes < 0 {
panic("MIPS: sub overflow")
}
if intRs < 0 && intRt > 0 && intRes > 0 {
panic("MIPS: sub underflow")
}
return uint32(intRes)
case 0x23: // subu
return rs - rt
case 0x24: // and
......
......@@ -5,6 +5,7 @@ import (
"io"
"os"
"path"
"strings"
"testing"
"time"
......@@ -48,6 +49,7 @@ func TestEVM(t *testing.T) {
oracle := testutil.SelectOracleFixture(t, f.Name())
// Short-circuit early for exit_group.bin
exitGroup := f.Name() == "exit_group.bin"
expectPanic := strings.HasSuffix(f.Name(), "panic.bin")
evm := testutil.NewMIPSEVM(contracts, addrs)
evm.SetTracer(tracer)
......@@ -66,6 +68,17 @@ func TestEVM(t *testing.T) {
goState := singlethreaded.NewInstrumentedState(state, oracle, os.Stdout, os.Stderr, nil)
// Catch panics and check if they are expected
defer func() {
if r := recover(); r != nil {
if expectPanic {
// Success
} else {
t.Errorf("unexpected panic: %v", r)
}
}
}()
for i := 0; i < 1000; i++ {
curStep := goState.GetState().GetStep()
if goState.GetState().GetPC() == testutil.EndAddr {
......
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
lui $t0, 0x7fff # A = 0x7fffffff (maximum positive 32-bit integer)
ori $t0, 0xffff
ori $t1, $0, 1 # B = 0x1
add $t2, $t0, $t1 # C = A + B (this should trigger an overflow)
# Unreachable ....
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
lui $t0, 0x8000 # A = 0x80000000 (minimum negative 32-bit integer)
ori $t0, 0x0000
lui $t1, 0x8000 # B = 0x80000000 (minimum negative 32-bit integer)
ori $t1, 0x0000
add $t2, $t0, $t1 # C = A + B (this should trigger an underflow)
# Unreachable ....
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
lui $t0, 0x7fff # A = 0x7fffffff (maximum positive 32-bit integer)
ori $t0, 0xffff
addi $t1, $t0, 1 # B = A + 1 (this should trigger an overflow)
# Unreachable...
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
lui $t0, 0x8000 # A = 0x80000000 (minimum negative 32-bit integer)
ori $t0, 0x0000
addi $t1, $t0, -1 # B = A - 1 (this should trigger an underflow)
# Unreachable...
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
lui $t0, 0xffff # A = 0xffffffff (maximum unsigned 32-bit integer)
ori $t0, 0xffff
ori $t1, $0, 1 # B = 1
addu $t2, $t0, $t1 # C = A + B (simulate overflow)
sltu $v0, $t2, $t0 # D = 1 if overflow (C < A)
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
lui $t0, 0x7fff # A = 0x7fffffff (maximum positive 32-bit integer)
ori $t0, 0xffff
lui $t1, 0xffff # B = 0xffffffff (-1)
ori $t1, 0xffff
sub $t2, $t0, $t1 # C = A - B = 0x7fffffff - (-1) = 0x80000000 (overflow)
# Unreachable...
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
lui $t0, 0x8000 # A = 0x80000000 (minimum negative 32-bit integer)
ori $t0, 0x0000
ori $t1, $0, 1 # B = 1
sub $t2, $t0, $t1 # C = A - B = 0x80000000 - 1 (underflow)
# Unreachable...
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
.section .test, "x"
.balign 4
.set noreorder
.global test
.ent test
test:
lui $s0, 0xbfff # Load the base address 0xbffffff0
ori $s0, 0xfff0
ori $s1, $0, 1 # Prepare the 'done' status
#### Test code start ####
ori $t0, $0, 1 # A = 1
ori $t1, $0, 2 # B = 2
subu $t2, $t0, $t1 # C = A - B = 0xFFFFFFFF
sltu $v0, $t0, $t1 # D = 1 if underflow occurred (A < B)
#### Test code end ####
sw $v0, 8($s0) # Set the test result
sw $s1, 4($s0) # Set 'done'
$done:
jr $ra
nop
.end test
......@@ -5,6 +5,7 @@ import (
"io"
"os"
"path"
"strings"
"testing"
"github.com/ethereum/go-ethereum/log"
......@@ -32,6 +33,7 @@ func RunVMTests_OpenMips[T mipsevm.FPVMState](t *testing.T, stateFactory StateFa
oracle := SelectOracleFixture(t, f.Name())
// Short-circuit early for exit_group.bin
exitGroup := f.Name() == "exit_group.bin"
expectPanic := strings.HasSuffix(f.Name(), "panic.bin")
// TODO: currently tests are compiled as flat binary objects
// We can use more standard tooling to compile them to ELF files and get remove maketests.py
......@@ -51,6 +53,17 @@ func RunVMTests_OpenMips[T mipsevm.FPVMState](t *testing.T, stateFactory StateFa
us := vmFactory(state, oracle, os.Stdout, os.Stderr, CreateLogger())
// Catch panics and check if they are expected
defer func() {
if r := recover(); r != nil {
if expectPanic {
// Success
} else {
t.Errorf("unexpected panic: %v", r)
}
}
}()
for i := 0; i < 1000; i++ {
if us.GetState().GetPC() == EndAddr {
break
......
......@@ -140,11 +140,11 @@
"sourceCodeHash": "0x3ff4a3f21202478935412d47fd5ef7f94a170402ddc50e5c062013ce5544c83f"
},
"src/cannon/MIPS.sol": {
"initCodeHash": "0xfc2381da4ad202f1ff7b97776be0e9a510ef3c36c0f557cbada1a0943f376e62",
"initCodeHash": "0x67e75c05afbb472210294f922631a3f5762b05216ba2351c493aa846e5b45dfc",
"sourceCodeHash": "0x5f5d13be282305b2947079cbf8f01e750e5c88a2b7a7a7267e79cba3ee4616fa"
},
"src/cannon/MIPS2.sol": {
"initCodeHash": "0xd75b7541ca736dff03b1fa7116a9835f97bc82507287bf90f3dd739299f5aa03",
"initCodeHash": "0x926af65d34d68a15333d127f5354ff2dcb6307fa51b799f597fd3c648b5c901e",
"sourceCodeHash": "0x115bd6a4c4d77ed210dfd468675b409fdae9f79b932063c138f0765ba9063462"
},
"src/cannon/PreimageOracle.sol": {
......
This source diff could not be displayed because it is too large. You can view the blob instead.
......@@ -291,7 +291,12 @@ library MIPSInstructions {
// The rest includes transformed R-type arith imm instructions
// add
else if (_fun == 0x20) {
return (_rs + _rt);
int32 rs = int32(_rs);
int32 rt = int32(_rt);
int32 res = rs + rt;
require(!(rs > 0 && rt > 0 && res <= 0), "MIPS: add overflow");
require(!(rs < 0 && rt < 0 && res >= 0), "MIPS: add underflow");
return uint32(res);
}
// addu
else if (_fun == 0x21) {
......@@ -299,7 +304,12 @@ library MIPSInstructions {
}
// sub
else if (_fun == 0x22) {
return (_rs - _rt);
int32 rs = int32(_rs);
int32 rt = int32(_rt);
int32 res = rs - rt;
require(!(rs > 0 && rt < 0 && res <= 0), "MIPS: sub overflow");
require(!(rs < 0 && rt > 0 && res >= 0), "MIPS: sub underflow");
return uint32(res);
}
// subu
else if (_fun == 0x23) {
......
......@@ -78,6 +78,42 @@ contract MIPS_Test is CommonTest {
assertEq(postState, outputState(expect), "unexpected post state");
}
/// @notice Tests that add overflow triggers a revert.
/// @param _rs The first operand.
/// @param _rt The second operand.
function testFuzz_add_overflow_fails(int32 _rs, int32 _rt) external {
// Force overflow.
_rs = int32(bound(int256(_rs), int256(1), int256(type(int32).max)));
_rt = int32(bound(int256(_rt), int256(type(int32).max) - int256(_rs) + 1, int256(type(int32).max)));
uint32 insn = encodespec(17, 18, 8, 0x20); // add t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[17] = uint32(_rs);
state.registers[18] = uint32(_rt);
bytes memory encodedState = encodeState(state);
vm.expectRevert("MIPS: add overflow");
mips.step(encodedState, proof, 0);
}
/// @notice Tests that add underflow triggers a revert.
/// @param _rs The first operand.
/// @param _rt The second operand.
function testFuzz_add_underflow_fails(int32 _rs, int32 _rt) external {
// Force underflow.
_rs = int32(bound(int256(_rs), int256(type(int32).min), int256(-1)));
_rt = int32(bound(int256(_rt), int256(type(int32).min), int256(type(int32).min) - int256(_rs) - 1));
uint32 insn = encodespec(17, 18, 8, 0x20); // add t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[17] = uint32(_rs);
state.registers[18] = uint32(_rt);
bytes memory encodedState = encodeState(state);
vm.expectRevert("MIPS: add underflow");
mips.step(encodedState, proof, 0);
}
function test_addu_succeeds() external {
uint32 insn = encodespec(17, 18, 8, 0x21); // addu t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
......@@ -98,6 +134,35 @@ contract MIPS_Test is CommonTest {
assertEq(postState, outputState(expect), "unexpected post state");
}
/// @notice Tests that addu overflow does not trigger a revert.
/// @param _rs The first operand.
/// @param _rt The second operand.
function testFuzz_addu_overflow_succeeds(uint32 _rs, uint32 _rt) external {
// Force overflow.
_rs = uint32(bound(_rs, 1, type(uint32).max));
_rt = uint32(bound(_rt, type(uint32).max - _rs + 1, type(uint32).max));
uint32 insn = encodespec(17, 18, 8, 0x21); // addu t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[17] = _rs;
state.registers[18] = _rt;
bytes memory encodedState = encodeState(state);
MIPS.State memory expect;
unchecked {
expect.memRoot = state.memRoot;
expect.pc = state.nextPC;
expect.nextPC = state.nextPC + 4;
expect.step = state.step + 1;
expect.registers[8] = state.registers[17] + state.registers[18]; // t0
expect.registers[17] = state.registers[17];
expect.registers[18] = state.registers[18];
}
bytes32 postState = mips.step(encodedState, proof, 0);
assertEq(postState, outputState(expect), "unexpected post state");
}
function test_addi_succeeds() external {
uint16 imm = 40;
uint32 insn = encodeitype(0x8, 17, 8, imm); // addi t0, s1, 40
......@@ -118,6 +183,42 @@ contract MIPS_Test is CommonTest {
assertEq(postState, outputState(expect), "unexpected post state");
}
/// @notice Tests that the addi overflow triggers a revert.
/// @param _rs The first operand.
/// @param _imm Immediate value.
function testFuzz_addi_overflow_fails(int32 _rs, int16 _imm) external {
// Force overflow.
_imm = int16(bound(int256(_imm), int256(1), int256(type(int16).max)));
_rs = int32(bound(int256(_rs), int256(type(int32).max) - int256(_imm) + 1, int256(type(int32).max)));
uint32 insn = encodeitype(0x8, 17, 8, uint16(_imm)); // addi t0, s1, _imm
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[8] = 1; // t0
state.registers[17] = uint32(_rs); // s1
bytes memory encodedState = encodeState(state);
vm.expectRevert("MIPS: add overflow");
mips.step(encodedState, proof, 0);
}
/// @notice Tests that addi underflow triggers a reverts.
/// @param _rs The first operand.
/// @param _imm Immediate value.
function testFuzz_addi_underflow_fails(int32 _rs, int16 _imm) external {
// Force underflow.
_rs = int32(bound(int256(_rs), int256(type(int32).min), int256(type(int32).min) + int256(type(int16).max) - 2));
_imm = int16(bound(int256(_imm), int256(type(int16).min), int256(type(int32).min) - int256(_rs) - 1));
uint32 insn = encodeitype(0x8, 17, 8, uint16(_imm)); // addi t0, s1, _imm
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[8] = 1; // t0
state.registers[17] = uint32(_rs); // s1
bytes memory encodedState = encodeState(state);
vm.expectRevert("MIPS: add underflow");
mips.step(encodedState, proof, 0);
}
function test_addiSign_succeeds() external {
uint16 imm = 0xfffe; // -2
uint32 insn = encodeitype(0x8, 17, 8, imm); // addi t0, s1, 40
......@@ -158,6 +259,34 @@ contract MIPS_Test is CommonTest {
assertEq(postState, outputState(expect), "unexpected post state");
}
/// @notice Tests that the addui overflow does not trigger a revert.
/// @param _rs The first operand.
/// @param _imm Immediate value.
function testFuzz_addui_overflow_succeeds(int32 _rs, int16 _imm) external {
// Force overflow.
_imm = int16(bound(int256(_imm), int256(1), int256(type(int16).max)));
_rs = int32(bound(int256(_rs), int256(type(int32).max) - int256(_imm) + 1, int256(type(int32).max)));
uint32 insn = encodeitype(0x9, 17, 8, uint16(_imm)); // addui t0, s1, _imm
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[8] = 1; // t0
state.registers[17] = uint32(_rs); // s1
bytes memory encodedState = encodeState(state);
MIPS.State memory expect;
unchecked {
expect.memRoot = state.memRoot;
expect.pc = state.nextPC;
expect.nextPC = state.nextPC + 4;
expect.step = state.step + 1;
expect.registers[8] = uint32(state.registers[17]) + uint16(_imm);
expect.registers[17] = uint32(state.registers[17]);
}
bytes32 postState = mips.step(encodedState, proof, 0);
assertEq(postState, outputState(expect), "unexpected post state");
}
function test_sub_succeeds() external {
uint32 insn = encodespec(17, 18, 8, 0x22); // sub t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
......@@ -178,6 +307,44 @@ contract MIPS_Test is CommonTest {
assertEq(postState, outputState(expect), "unexpected post state");
}
/// @notice Tests that the sub overflow triggers a revert.
/// @param _rs The first operand.
/// @param _rt The second operand.
function testFuzz_sub_overflow_fails(int32 _rs, int32 _rt) external {
// Force overflow.
_rs = int32(bound(int256(_rs), int256(1), int256(type(int32).max)));
_rt = int32(bound(int256(_rt), int256(type(int32).min), int256(_rs) - int256(type(int32).max) - 1));
uint32 insn = encodespec(17, 18, 8, 0x22); // sub t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[17] = uint32(_rs);
state.registers[18] = uint32(_rt);
bytes memory encodedState = encodeState(state);
vm.expectRevert("MIPS: sub overflow");
mips.step(encodedState, proof, 0);
}
/// @notice Tests that the sub underflow triggers a revert.
/// @param _rs The first operand.
/// @param _rt The second operand.
function testFuzz_sub_underflow_fails(int32 _rs, int32 _rt) external {
// Force underflow.
// Minimum value for int32 is -2^31 but the maximum value is 2^31 - 1. We therefore cannot
// trigger an underflow for any number >= -1 because -1 - (2^31 - 1) = -2^31.
_rs = int32(bound(int256(_rs), int256(type(int32).min), int256(-2)));
_rt = int32(bound(int256(_rt), int256(type(int32).max) + 2 + int256(_rs), int256(type(int32).max)));
uint32 insn = encodespec(17, 18, 8, 0x22); // sub t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[17] = uint32(_rs);
state.registers[18] = uint32(_rt);
bytes memory encodedState = encodeState(state);
vm.expectRevert("MIPS: sub underflow");
mips.step(encodedState, proof, 0);
}
function test_subu_succeeds() external {
uint32 insn = encodespec(17, 18, 8, 0x23); // subu t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
......@@ -198,6 +365,35 @@ contract MIPS_Test is CommonTest {
assertEq(postState, outputState(expect), "unexpected post state");
}
/// @notice Tests that subu overflow does not trigger a revert.
/// @param _rs The first operand.
/// @param _rt The second operand.
function testFuzz_subu_underflow_succeeds(uint32 _rs, uint32 _rt) external {
// Force underflow.
_rs = uint32(bound(_rs, 0, type(uint32).max - 1));
_rt = uint32(bound(_rt, type(uint32).max - _rs, type(uint32).max));
uint32 insn = encodespec(17, 18, 8, 0x23); // subu t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
state.registers[17] = _rs;
state.registers[18] = _rt;
bytes memory encodedState = encodeState(state);
MIPS.State memory expect;
unchecked {
expect.memRoot = state.memRoot;
expect.pc = state.nextPC;
expect.nextPC = state.nextPC + 4;
expect.step = state.step + 1;
expect.registers[8] = state.registers[17] - state.registers[18]; // t0
expect.registers[17] = state.registers[17];
expect.registers[18] = state.registers[18];
}
bytes32 postState = mips.step(encodedState, proof, 0);
assertEq(postState, outputState(expect), "unexpected post state");
}
function test_and_succeeds() external {
uint32 insn = encodespec(17, 18, 8, 0x24); // and t0, s1, s2
(MIPS.State memory state, bytes memory proof) = constructMIPSState(0, insn, 0x4, 0);
......
......@@ -27,7 +27,7 @@ contract DeploymentSummary is DeploymentSummaryCode {
address internal constant l1StandardBridgeProxyAddress = 0x20A42a5a785622c6Ba2576B2D6e924aA82BFA11D;
address internal constant l2OutputOracleAddress = 0x19652082F846171168Daf378C4fD3ee85a0D4A60;
address internal constant l2OutputOracleProxyAddress = 0x39Af23E00F1e662025aA01b0cEdA19542B78DF99;
address internal constant mipsAddress = 0x1620d4F04388901B3a12303df1F235Bb0C92B53D;
address internal constant mipsAddress = 0xecD4f82974C78282709e1C318ef108AA5c4631D3;
address internal constant optimismMintableERC20FactoryAddress = 0x39Aea2Dd53f2d01c15877aCc2791af6BDD7aD567;
address internal constant optimismMintableERC20FactoryProxyAddress = 0xc7B87b2b892EA5C3CfF47168881FE168C00377FB;
address internal constant optimismPortalAddress = 0xbdD90485FCbcac869D5b5752179815a3103d8131;
......
This source diff could not be displayed because it is too large. You can view the blob instead.
......@@ -27,7 +27,7 @@ contract DeploymentSummaryFaultProofs is DeploymentSummaryFaultProofsCode {
address internal constant l1StandardBridgeProxyAddress = 0x20A42a5a785622c6Ba2576B2D6e924aA82BFA11D;
address internal constant l2OutputOracleAddress = 0x19652082F846171168Daf378C4fD3ee85a0D4A60;
address internal constant l2OutputOracleProxyAddress = 0x39Af23E00F1e662025aA01b0cEdA19542B78DF99;
address internal constant mipsAddress = 0x1620d4F04388901B3a12303df1F235Bb0C92B53D;
address internal constant mipsAddress = 0xecD4f82974C78282709e1C318ef108AA5c4631D3;
address internal constant optimismMintableERC20FactoryAddress = 0x39Aea2Dd53f2d01c15877aCc2791af6BDD7aD567;
address internal constant optimismMintableERC20FactoryProxyAddress = 0xc7B87b2b892EA5C3CfF47168881FE168C00377FB;
address internal constant optimismPortalAddress = 0xbdD90485FCbcac869D5b5752179815a3103d8131;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment