From aab700d25df795eb695b0e5f7d9e39e4c1a6e30b Mon Sep 17 00:00:00 2001 From: Jochem Brouwer Date: Thu, 17 Sep 2020 11:44:52 +0200 Subject: [PATCH] [VM] StateManager: strip leading zeros, throw on more than 32 byte values in putContractStorage --- packages/vm/lib/state/stateManager.ts | 12 +++- packages/vm/tests/api/state/stateManager.js | 79 ++++++++++++++++++++- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/packages/vm/lib/state/stateManager.ts b/packages/vm/lib/state/stateManager.ts index 3e5d5db7b98..0fda379c58e 100644 --- a/packages/vm/lib/state/stateManager.ts +++ b/packages/vm/lib/state/stateManager.ts @@ -1,6 +1,6 @@ const Set = require('core-js-pure/es/set') import { SecureTrie as Trie } from 'merkle-patricia-tree' -import { BN, toBuffer, keccak256, KECCAK256_NULL } from 'ethereumjs-util' +import { BN, toBuffer, keccak256, KECCAK256_NULL, unpadBuffer } from 'ethereumjs-util' import { encode, decode } from 'rlp' import Common from '@ethereumjs/common' import { genesisStateByName } from '@ethereumjs/common/dist/genesisStates' @@ -276,13 +276,19 @@ export default class DefaultStateManager implements StateManager { * corresponding to `address` at the provided `key`. * @param address - Address to set a storage value for * @param key - Key to set the value at. Must be 32 bytes long. - * @param value - Value to set at `key` for account corresponding to `address` + * @param value - Value to set at `key` for account corresponding to `address`. Cannot be more than 32 bytes. Will be left-padded with zeros if less than 32 bytes. If it is a empty or filled with zeros, delete the value. */ async putContractStorage(address: Buffer, key: Buffer, value: Buffer): Promise { if (key.length !== 32) { throw new Error('Storage key must be 32 bytes long') } + if (value.length > 32) { + throw new Error('Storage key cannot be longer than 32 bytes') + } + + value = unpadBuffer(value) + await this._modifyContractStorage(address, async (storageTrie, done) => { if (value && value.length) { // format input @@ -415,7 +421,7 @@ export default class DefaultStateManager implements StateManager { } /** - * Dumps the the storage values for an `account` specified by `address`. + * Dumps the the RLP-encoded storage values for an `account` specified by `address`. * @param address - The address of the `account` to return storage for * @returns {Promise} - The state of the account as an `Object` map. * Keys are are the storage keys, values are the storage values as strings. diff --git a/packages/vm/tests/api/state/stateManager.js b/packages/vm/tests/api/state/stateManager.js index 214003f8095..58c882ff262 100644 --- a/packages/vm/tests/api/state/stateManager.js +++ b/packages/vm/tests/api/state/stateManager.js @@ -1,5 +1,5 @@ const tape = require('tape') -const { toBuffer, keccak256, KECCAK256_RLP } = require('ethereumjs-util') +const { toBuffer, keccak256, KECCAK256_RLP, unpadBuffer, zeros } = require('ethereumjs-util') const Common = require('@ethereumjs/common').default const Account = require('@ethereumjs/account').default const { DefaultStateManager } = require('../../../dist/state') @@ -420,3 +420,80 @@ tape('StateManager - Contract code', (tester) => { t.end() }) }) + +tape('StateManager - Contract storage', (tester) => { + const it = tester.test + + it('should throw on storage values larger than 32 bytes', async(t) => { + t.plan(1) + const stateManager = new DefaultStateManager() + const address = zeros(20) + const key = zeros(32) + const value = Buffer.from(("aa").repeat(33), 'hex') + try { + await stateManager.putContractStorage(address, key, value) + t.fail("did not throw") + } catch (e) { + t.pass("threw on trying to set storage values larger than 32 bytes") + } + t.end() + }) + + it('should strip storage values lower than 32 bytes', async(t) => { + const stateManager = new DefaultStateManager() + const address = zeros(20) + const key0 = zeros(32) + const value0 = Buffer.from(("aa").repeat(31), 'hex') // put a value of 31-bytes length + const expect0 = unpadBuffer(value0) + const key1 = Buffer.concat([zeros(31), Buffer.from('01', 'hex')]) + const value1 = Buffer.from(("aa").repeat(1), 'hex') // put a vlaue of 1-byte length + const expect1 = unpadBuffer(value1) + + await stateManager.putContractStorage(address, key0, value0) + const slot0 = await stateManager.getContractStorage(address, key0) + t.ok(slot0.equals(expect0), 'value of 31 bytes padded correctly') + + await stateManager.putContractStorage(address, key1, value1) + const slot1 = await stateManager.getContractStorage(address, key1) + t.ok(slot1.equals(expect1), 'value of 1 byte padded correctly') + t.end() + }) + + it ('should delete storage values which only consist of zero bytes', async(t) => { + const address = zeros(20) + const key = zeros(32) + const startValue = Buffer.from('01', 'hex') + + const zeroLengths = [0, 1, 31, 32] // checks for arbitrary-length zeros + t.plan(zeroLengths.length) + + for (let length of zeroLengths) { + const stateManager = new DefaultStateManager() + const value = zeros(length) + await stateManager.putContractStorage(address, key, startValue) + const currentValue = await stateManager.getContractStorage(address, key) + if (!currentValue.equals(startValue)) { + // sanity check + t.fail("contract value not set correctly") + } else { + // delete the value + await stateManager.putContractStorage(address, key, value) + const deleted = await stateManager.getContractStorage(address, key) + t.ok(deleted.equals(zeros(0)), "the storage key should be deleted") + } + } + t.end() + }) + + it('should not strip trailing zeros', async(t) => { + const address = zeros(20) + const key = zeros(32) + const value = Buffer.from('0000aabb00', 'hex') + const expect = Buffer.from('aabb00', 'hex') + const stateManager = new DefaultStateManager() + await stateManager.putContractStorage(address, key, value) + const contractValue = await stateManager.getContractStorage(address, key) + t.ok(contractValue.equals(expect), 'trailing zeros are not stripped') + t.end() + }) +}) \ No newline at end of file