Skip to content

Commit

Permalink
Fix the vulnerability found in issue #3861 (#3863)
Browse files Browse the repository at this point in the history
* Fix PUSHN Opcode Non-Compliance

Co-authored-by: Lexterl33t <bryton@fuzzinglabs.com>

* Test for PUSHn non compliance

Co-authored-by: mhoste51 <mathieu@fuzzinglabs.com>

* 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 <mathieu@fuzzinglabs.com>

* 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 <mathieu@fuzzinglabs.com>
Co-authored-by: Scorbajio <indigophi@protonmail.com>
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
  • Loading branch information
4 people authored Feb 10, 2025
1 parent 1774df6 commit a1f5de0
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 7 deletions.
11 changes: 8 additions & 3 deletions packages/evm/src/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
bytesToBigInt,
bytesToHex,
equalsBytes,
setLengthRight,
} from '@ethereumjs/util'
import debugDefault from 'debug'

Expand Down Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions packages/evm/src/opcodes/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
bytesToInt,
concatBytes,
setLengthLeft,
setLengthRight,
} from '@ethereumjs/util'
import { keccak256 } from 'ethereum-cryptography/keccak.js'

Expand Down Expand Up @@ -935,11 +936,16 @@ export const handlers: Map<number, OpHandler> = 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))
}
},
],
Expand Down
36 changes: 35 additions & 1 deletion packages/evm/test/stack.spec.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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')
}
})
})

0 comments on commit a1f5de0

Please sign in to comment.