Skip to content

Commit

Permalink
fix(EVM): Do not check memory boundaries if size is zero (#1114)
Browse files Browse the repository at this point in the history
  • Loading branch information
0xVolosnikov committed Dec 10, 2024
1 parent fd14aaf commit e014caa
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 45 deletions.
84 changes: 54 additions & 30 deletions system-contracts/contracts/EvmEmulator.yul
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,12 @@ object "EvmEmulator" {
}

function checkMemIsAccessible(relativeOffset, size) {
checkOverflow(relativeOffset, size)
if size {
checkOverflow(relativeOffset, size)

if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
panic()
if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
panic()
}
}
}

Expand Down Expand Up @@ -2676,14 +2678,17 @@ object "EvmEmulator" {
offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

checkMemIsAccessible(offset, size)
if size {
checkMemIsAccessible(offset, size)

evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

returnLen := size

// Don't check overflow here since previous checks are enough to ensure this is safe
returnOffset := add(MEM_OFFSET(), offset)
}

returnLen := size

// Don't check overflow here since previous checks are enough to ensure this is safe
returnOffset := add(MEM_OFFSET(), offset)
break
}
case 0xF4 { // OP_DELEGATECALL
Expand All @@ -2710,12 +2715,19 @@ object "EvmEmulator" {
popStackCheck(sp, 2)
offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

checkMemIsAccessible(offset, size)
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

// Don't check overflow here since previous checks are enough to ensure this is safe
offset := add(offset, MEM_OFFSET())

switch iszero(size)
case 0 {
checkMemIsAccessible(offset, size)
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

// Don't check overflow here since previous checks are enough to ensure this is safe
offset := add(offset, MEM_OFFSET())
}
default {
offset := MEM_OFFSET()
}


if eq(isCallerEVM, 1) {
offset := sub(offset, 32)
Expand Down Expand Up @@ -3340,10 +3352,12 @@ object "EvmEmulator" {
}

function checkMemIsAccessible(relativeOffset, size) {
checkOverflow(relativeOffset, size)
if size {
checkOverflow(relativeOffset, size)

if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
panic()
if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
panic()
}
}
}

Expand Down Expand Up @@ -5736,14 +5750,17 @@ object "EvmEmulator" {
offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

checkMemIsAccessible(offset, size)
if size {
checkMemIsAccessible(offset, size)

evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

returnLen := size

// Don't check overflow here since previous checks are enough to ensure this is safe
returnOffset := add(MEM_OFFSET(), offset)
}

returnLen := size

// Don't check overflow here since previous checks are enough to ensure this is safe
returnOffset := add(MEM_OFFSET(), offset)
break
}
case 0xF4 { // OP_DELEGATECALL
Expand All @@ -5770,12 +5787,19 @@ object "EvmEmulator" {
popStackCheck(sp, 2)
offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

checkMemIsAccessible(offset, size)
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

// Don't check overflow here since previous checks are enough to ensure this is safe
offset := add(offset, MEM_OFFSET())

switch iszero(size)
case 0 {
checkMemIsAccessible(offset, size)
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

// Don't check overflow here since previous checks are enough to ensure this is safe
offset := add(offset, MEM_OFFSET())
}
default {
offset := MEM_OFFSET()
}


if eq(isCallerEVM, 1) {
offset := sub(offset, 32)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,12 @@ function expandMemory2(retOffset, retSize, argsOffset, argsSize) -> maxExpand {
}

function checkMemIsAccessible(relativeOffset, size) {
checkOverflow(relativeOffset, size)
if size {
checkOverflow(relativeOffset, size)

if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
panic()
if gt(add(relativeOffset, size), MAX_POSSIBLE_MEM_LEN()) {
panic()
}
}
}

Expand Down
34 changes: 22 additions & 12 deletions system-contracts/evm-emulator/EvmEmulatorLoop.template.yul
Original file line number Diff line number Diff line change
Expand Up @@ -1454,14 +1454,17 @@ for { } true { } {
offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

checkMemIsAccessible(offset, size)
if size {
checkMemIsAccessible(offset, size)

evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

returnLen := size

// Don't check overflow here since previous checks are enough to ensure this is safe
returnOffset := add(MEM_OFFSET(), offset)
}

returnLen := size

// Don't check overflow here since previous checks are enough to ensure this is safe
returnOffset := add(MEM_OFFSET(), offset)
break
}
case 0xF4 { // OP_DELEGATECALL
Expand All @@ -1488,12 +1491,19 @@ for { } true { } {
popStackCheck(sp, 2)
offset, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)
size, sp, stackHead := popStackItemWithoutCheck(sp, stackHead)

checkMemIsAccessible(offset, size)
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

// Don't check overflow here since previous checks are enough to ensure this is safe
offset := add(offset, MEM_OFFSET())

switch iszero(size)
case 0 {
checkMemIsAccessible(offset, size)
evmGasLeft := chargeGas(evmGasLeft, expandMemory(offset, size))

// Don't check overflow here since previous checks are enough to ensure this is safe
offset := add(offset, MEM_OFFSET())
}
default {
offset := MEM_OFFSET()
}


if eq(isCallerEVM, 1) {
offset := sub(offset, 32)
Expand Down

0 comments on commit e014caa

Please sign in to comment.