From a1f5de03da88ef59fc7ba9d80b42857986f57ce5 Mon Sep 17 00:00:00 2001 From: Lxt3h <44911576+Lexterl33t@users.noreply.github.com> Date: Mon, 10 Feb 2025 19:58:11 +0100 Subject: [PATCH] Fix the vulnerability found in issue #3861 (#3863) * Fix PUSHN Opcode Non-Compliance Co-authored-by: Lexterl33t * Test for PUSHn non compliance Co-authored-by: mhoste51 * Remove console.log from test * Comment moved above the code * Comment moved above the code (updated) * Update the user balance to 0 * Refacto after the review Co-authored-by: mhoste51 * evm: update push logic * evm: test all push opcodes * evm: add stack test for code with jumps, truncated push opcodes * evm: fix cache item for truncated pushs * evm/interpreter: use subarray instead of slice for push bytes cache --------- Co-authored-by: Mhoste Co-authored-by: Scorbajio Co-authored-by: Jochem Brouwer --- packages/evm/src/interpreter.ts | 11 +++++--- packages/evm/src/opcodes/functions.ts | 12 ++++++--- packages/evm/test/stack.spec.ts | 36 ++++++++++++++++++++++++++- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/packages/evm/src/interpreter.ts b/packages/evm/src/interpreter.ts index 72e2657d30..eda6ae09d3 100644 --- a/packages/evm/src/interpreter.ts +++ b/packages/evm/src/interpreter.ts @@ -9,6 +9,7 @@ import { bytesToBigInt, bytesToHex, equalsBytes, + setLengthRight, } from '@ethereumjs/util' import debugDefault from 'debug' @@ -521,10 +522,14 @@ export class Interpreter { // skip over PUSH0-32 since no jump destinations in the middle of a push block if (opcode <= 0x7f) { if (opcode >= 0x60) { - const extraSteps = opcode - 0x5f - const push = bytesToBigInt(code.slice(i + 1, i + opcode - 0x5e)) + const bytesToPush = opcode - 0x5f + let pushBytes = code.subarray(i + 1, i + opcode - 0x5e) + if (pushBytes.length < bytesToPush) { + pushBytes = setLengthRight(pushBytes, bytesToPush) + } + const push = bytesToBigInt(pushBytes) pushes[i + 1] = push - i += extraSteps + i += bytesToPush } else if (opcode === 0x5b) { // Define a JUMPDEST as a 1 in the valid jumps array jumps[i] = 1 diff --git a/packages/evm/src/opcodes/functions.ts b/packages/evm/src/opcodes/functions.ts index 00abcc02b1..544c081aa1 100644 --- a/packages/evm/src/opcodes/functions.ts +++ b/packages/evm/src/opcodes/functions.ts @@ -24,6 +24,7 @@ import { bytesToInt, concatBytes, setLengthLeft, + setLengthRight, } from '@ethereumjs/util' import { keccak256 } from 'ethereum-cryptography/keccak.js' @@ -935,11 +936,16 @@ export const handlers: Map = new Map([ runState.stack.push(runState.cachedPushes[runState.programCounter]) runState.programCounter += numToPush } else { - const loaded = bytesToBigInt( - runState.code.subarray(runState.programCounter, runState.programCounter + numToPush), + let loadedBytes = runState.code.subarray( + runState.programCounter, + runState.programCounter + numToPush, ) + if (loadedBytes.length < numToPush) { + loadedBytes = setLengthRight(loadedBytes, numToPush) + } + runState.programCounter += numToPush - runState.stack.push(loaded) + runState.stack.push(bytesToBigInt(loadedBytes)) } }, ], diff --git a/packages/evm/test/stack.spec.ts b/packages/evm/test/stack.spec.ts index ed75419f04..b759c02e24 100644 --- a/packages/evm/test/stack.spec.ts +++ b/packages/evm/test/stack.spec.ts @@ -1,4 +1,12 @@ -import { Account, Address, bigIntToBytes, hexToBytes, setLengthLeft } from '@ethereumjs/util' +import { + Account, + Address, + bigIntToBytes, + bytesToBigInt, + hexToBytes, + setLengthLeft, + setLengthRight, +} from '@ethereumjs/util' import { assert, describe, it } from 'vitest' import { createEVM } from '../src/index.js' @@ -148,4 +156,30 @@ describe('Stack', () => { const reportedStack = s.getStack() assert.deepEqual(reportedStack, [BigInt(4), BigInt(6)]) }) + + it('stack should return the padded value', async () => { + const evm = await createEVM() + + for (let pushN = 0x60; pushN <= 0x7f; pushN++) { + // PUSHx 01 + const code = `0x${pushN.toString(16)}01` + // PUSH 0x03 JUMP JUMPDEST < PUSHx 01 > + const codeWithJumps = `0x6003565B${pushN.toString(16)}01` + + const expectedStack = new Stack(1024) + expectedStack.push(bytesToBigInt(setLengthRight(new Uint8Array([0x01]), pushN - 0x5f))) + + const resWithoutJumps = await evm.runCall({ + data: hexToBytes(code), + }) + const executionStack = resWithoutJumps.execResult.runState?.stack + assert.deepEqual(executionStack, expectedStack, 'code without jumps ok') + + const resWithJumps = await evm.runCall({ + data: hexToBytes(codeWithJumps), + }) + const executionStackWithJumps = resWithJumps.execResult.runState?.stack + assert.deepEqual(executionStackWithJumps, expectedStack, 'code with jumps ok') + } + }) })