Merge pull request #106 from ethereum-optimism/feature/mininny/audit-19

Correctly check memory alignment based on size of operand
ethereum-optimism · Jan 15, 2025 · 1ea4ef9 · 1ea4ef9
2 parents 606c94b + b7dc265
commit 1ea4ef9
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 9 deletions.
diff --git a/rvgo/fast/vm.go b/rvgo/fast/vm.go
@@ -887,7 +887,7 @@ func (inst *InstrumentedState) riscvStep() (outErr error) {
 			revertWithCode(riscv.ErrBadAMOSize, fmt.Errorf("bad AMO size: %d", size))
 		}
 		addr := getRegister(rs1)
-		if addr&3 != 0 { // quick addr alignment check
+		if mod64(addr, size) != 0 { // quick addr alignment check
 			revertWithCode(riscv.ErrNotAlignedAddr, fmt.Errorf("addr %d not aligned with 4 bytes", addr))
 		}
 

diff --git a/rvgo/slow/vm.go b/rvgo/slow/vm.go
@@ -1071,7 +1071,7 @@ func Step(calldata []byte, po PreimageOracle) (stateHash common.Hash, outErr err
 			revertWithCode(riscv.ErrBadAMOSize, fmt.Errorf("bad AMO size: %d", size))
 		}
 		addr := getRegister(rs1)
-		if and64(addr, toU64(3)) != (U64{}) { // quick addr alignment check
+		if mod64(addr, size) != (U64{}) { // quick addr alignment check
 			revertWithCode(riscv.ErrNotAlignedAddr, fmt.Errorf("addr %d not aligned with 4 bytes", addr))
 		}
 

diff --git a/rvsol/src/RISCV.sol b/rvsol/src/RISCV.sol
@@ -1575,7 +1575,7 @@ contract RISCV is IBigStepper {
                 if or(lt64(size, toU64(4)), gt64(size, toU64(8))) { revertWithCode(0xbada70) } // bad AMO size
 
                 let addr := getRegister(rs1)
-                if and64(addr, toU64(3)) {
+                if mod64(addr, size) {
                     // quick addr alignment check
                     revertWithCode(0xbad10ad0) // addr not aligned with 4 bytes
                 }

diff --git a/rvsol/test/RISCV.t.sol b/rvsol/test/RISCV.t.sol
@@ -989,7 +989,7 @@ contract RISCV_Test is CommonTest {
 
     function test_lrd_succeeds() public {
         bytes32 value = hex"a0b1df92a49eec39";
-        uint64 addr = 0xb86a394544c084ec;
+        uint64 addr = 0xb86a394544c084e0;
         uint8 funct3 = 0x3;
         uint8 funct7 = encodeFunct7(0x2, 0x0, 0x0);
         uint8 size = uint8(1 << (funct3 & 0x3));
@@ -1069,7 +1069,7 @@ contract RISCV_Test is CommonTest {
     }
 
     function test_amoaddd_succeeds() public {
-        uint64 addr = 0xeae426a36ff2bb64;
+        uint64 addr = 0xeae426a36ff2bb60;
         uint32 insn;
         uint8 size;
         {
@@ -1101,7 +1101,7 @@ contract RISCV_Test is CommonTest {
     }
 
     function test_amoxord_succeeds() public {
-        uint64 addr = 0x2d5ba68f57f1c564;
+        uint64 addr = 0x2d5ba68f57f1c560;
         uint32 insn;
         uint8 size;
         {
@@ -1164,7 +1164,7 @@ contract RISCV_Test is CommonTest {
     }
 
     function test_amoord_succeeds() public {
-        uint64 addr = 0xa0d7a5ea65b35664;
+        uint64 addr = 0xa0d7a5ea65b35660;
         uint32 insn;
         uint8 size;
         {
@@ -1260,7 +1260,7 @@ contract RISCV_Test is CommonTest {
     }
 
     function test_amominud_succeeds() public {
-        uint64 addr = 0xe094be571f4baca4;
+        uint64 addr = 0xe094be571f4baca0;
         uint32 insn;
         uint8 size;
         {
@@ -2440,7 +2440,7 @@ contract RISCV_Test is CommonTest {
     }
 
     function test_unknown_atomic_operation() public {
-        uint64 addr = 0xeae426a36ff2bb64;
+        uint64 addr = 0xeae426a36ff2bb68;
         uint32 insn;
         uint8 size;
         {