From d744a8fb483999fb16f95aa33d7606103ef6a1e6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 9 Nov 2020 15:43:57 +0100 Subject: [PATCH] Fail on invalid storage encoding for byte arrays. --- docs/control-structures.rst | 1 + libsolidity/codegen/ABIFunctions.cpp | 7 +- libsolidity/codegen/ArrayUtils.cpp | 65 +++++-------- libsolidity/codegen/YulUtilFunctions.cpp | 42 +++++---- libsolidity/codegen/YulUtilFunctions.h | 10 +- libsolutil/ErrorCodes.h | 1 + ...nvalid_encoding_for_storage_byte_array.sol | 92 +++++++++++++++++++ 7 files changed, 150 insertions(+), 68 deletions(-) create mode 100644 test/libsolidity/semanticTests/array/invalid_encoding_for_storage_byte_array.sol diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 88307d4bb..8aec32933 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -598,6 +598,7 @@ The error code supplied with the error data indicates the kind of panic. #. 0x11: If an arithmetic operation results in underflow or overflow outside of an ``unchecked { ... }`` block. #. 0x12; If you divide or modulo by zero (e.g. ``5 / 0`` or ``23 % 0``). #. 0x21: If you convert a value that is too big or negative into an enum type. +#. 0x22: If you access a storage byte array that is incorrectly encoded. #. 0x31: If you call ``.pop()`` on an empty array. #. 0x32: If you access an array, ``bytesN`` or an array slice at an out-of-bounds or negative index (i.e. ``x[i]`` where ``i >= x.length`` or ``i < 0``). #. 0x41: If you allocate too much memory or create an array that is too large. diff --git a/libsolidity/codegen/ABIFunctions.cpp b/libsolidity/codegen/ABIFunctions.cpp index 88e3c39fd..f817ae88d 100644 --- a/libsolidity/codegen/ABIFunctions.cpp +++ b/libsolidity/codegen/ABIFunctions.cpp @@ -683,18 +683,16 @@ string ABIFunctions::abiEncodingFunctionCompactStorageArray( // -> function (value, pos) -> ret { let slotValue := sload(value) + let length := (slotValue) + pos := (pos, length) switch and(slotValue, 1) case 0 { // short byte array - let length := and(div(slotValue, 2), 0x7f) - pos := (pos, length) mstore(pos, and(slotValue, not(0xff))) ret := add(pos, ) } case 1 { // long byte array - let length := div(slotValue, 2) - pos := (pos, length) let dataPos := (value) let i := 0 for { } lt(i, length) { i := add(i, 0x20) } { @@ -708,6 +706,7 @@ string ABIFunctions::abiEncodingFunctionCompactStorageArray( templ("functionName", functionName); templ("readableTypeNameFrom", _from.toString(true)); templ("readableTypeNameTo", _to.toString(true)); + templ("byteArrayLengthFunction", m_utils.extractByteArrayLengthFunction()); templ("storeLength", arrayStoreLengthForEncodingFunction(_to, _options)); templ("lengthPaddedShort", _options.padded ? "0x20" : "length"); templ("lengthPaddedLong", _options.padded ? "i" : "length"); diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 8554621bf..c67dd6ffb 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -818,30 +818,25 @@ void ArrayUtils::incrementDynamicArraySize(ArrayType const& _type) const // lowest-order byte (we actually use a mask with fewer bits) must // be (31*2+0) = 62 + m_context << Instruction::DUP1 << Instruction::SLOAD << Instruction::DUP1; + m_context.callYulFunction(m_context.utilFunctions().extractByteArrayLengthFunction(), 1, 1); m_context.appendInlineAssembly(R"({ - let data := sload(ref) - let shifted_length := and(data, 63) // We have to copy if length is exactly 31, because that marks // the transition between in-place and out-of-place storage. - switch shifted_length - case 62 + switch length + case 31 { mstore(0, ref) let data_area := keccak256(0, 0x20) sstore(data_area, and(data, not(0xff))) - // New length is 32, encoded as (32 * 2 + 1) - sstore(ref, 65) - // Replace ref variable by new length - ref := 32 + // Set old length in new format (31 * 2 + 1) + data := 63 } - default - { - sstore(ref, add(data, 2)) - // Replace ref variable by new length - if iszero(and(data, 1)) { data := shifted_length } - ref := add(div(data, 2), 1) - } - })", {"ref"}); + sstore(ref, add(data, 2)) + // return new length in ref + ref := add(length, 1) + })", {"ref", "data", "length"}); + m_context << Instruction::POP << Instruction::POP; } else m_context.appendInlineAssembly(R"({ @@ -860,28 +855,25 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const if (_type.isByteArray()) { + m_context << Instruction::DUP1 << Instruction::SLOAD << Instruction::DUP1; + m_context.callYulFunction(m_context.utilFunctions().extractByteArrayLengthFunction(), 1, 1); util::Whiskers code(R"({ - let slot_value := sload(ref) - switch and(slot_value, 1) + if iszero(length) { + mstore(0, ) + mstore(4, ) + revert(0, 0x24) + } + switch gt(length, 31) case 0 { // short byte array - let length := and(div(slot_value, 2), 0x1f) - if iszero(length) { - mstore(0, ) - mstore(4, ) - revert(0, 0x24) - } - // Zero-out the suffix including the least significant byte. let mask := sub(exp(0x100, sub(33, length)), 1) length := sub(length, 1) slot_value := or(and(not(mask), slot_value), mul(length, 2)) - sstore(ref, slot_value) } case 1 { // long byte array mstore(0, ref) - let length := div(slot_value, 2) let slot := keccak256(0, 0x20) switch length case 32 @@ -889,7 +881,7 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const let data := sload(slot) sstore(slot, 0) data := and(data, not(0xff)) - sstore(ref, or(data, 62)) + slot_value := or(data, 62) } default { @@ -905,14 +897,14 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const // Reduce the length by 1 slot_value := sub(slot_value, 2) - sstore(ref, slot_value) } } + sstore(ref, slot_value) })"); code("panicSelector", util::selectorFromSignature("Panic(uint256)").str()); code("emptyArrayPop", to_string(unsigned(util::PanicCode::EmptyArrayPop))); - m_context.appendInlineAssembly(code.render(), {"ref"}); - m_context << Instruction::POP; + m_context.appendInlineAssembly(code.render(), {"ref", "slot_value", "length"}); + m_context << Instruction::POP << Instruction::POP << Instruction::POP; } else { @@ -1040,16 +1032,7 @@ void ArrayUtils::retrieveLength(ArrayType const& _arrayType, unsigned _stackDept case DataLocation::Storage: m_context << Instruction::SLOAD; if (_arrayType.isByteArray()) - { - // Retrieve length both for in-place strings and off-place strings: - // Computes (x & (0x100 * (ISZERO (x & 1)) - 1)) / 2 - // i.e. for short strings (x & 1 == 0) it does (x & 0xff) / 2 and for long strings it - // computes (x & (-1)) / 2, which is equivalent to just x / 2. - m_context << u256(1) << Instruction::DUP2 << u256(1) << Instruction::AND; - m_context << Instruction::ISZERO << u256(0x100) << Instruction::MUL; - m_context << Instruction::SUB << Instruction::AND; - m_context << u256(2) << Instruction::SWAP1 << Instruction::DIV; - } + m_context.callYulFunction(m_context.utilFunctions().extractByteArrayLengthFunction(), 1, 1); break; } } diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index 402766612..7f1e08b8a 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -1003,25 +1003,6 @@ string YulUtilFunctions::wrappingIntExpFunction( }); } -string YulUtilFunctions::extractByteArrayLengthFunction() -{ - string functionName = "extract_byte_array_length"; - return m_functionCollector.createFunction(functionName, [&]() { - Whiskers w(R"( - function (data) -> length { - // Retrieve length both for in-place strings and off-place strings: - // Computes (x & (0x100 * (ISZERO (x & 1)) - 1)) / 2 - // i.e. for short strings (x & 1 == 0) it does (x & 0xff) / 2 and for long strings it - // computes (x & (-1)) / 2, which is equivalent to just x / 2. - let mask := sub(mul(0x100, iszero(and(data, 1))), 1) - length := div(and(data, mask), 2) - } - )"); - w("functionName", functionName); - return w.render(); - }); -} - string YulUtilFunctions::arrayLengthFunction(ArrayType const& _type) { string functionName = "array_length_" + _type.identifier(); @@ -1064,6 +1045,29 @@ string YulUtilFunctions::arrayLengthFunction(ArrayType const& _type) }); } +string YulUtilFunctions::extractByteArrayLengthFunction() +{ + string functionName = "extract_byte_array_length"; + return m_functionCollector.createFunction(functionName, [&]() { + Whiskers w(R"( + function (data) -> length { + length := div(data, 2) + let outOfPlaceEncoding := and(data, 1) + if iszero(outOfPlaceEncoding) { + length := and(length, 0x7f) + } + + if eq(outOfPlaceEncoding, lt(length, 32)) { + () + } + } + )"); + w("functionName", functionName); + w("panic", panicFunction(PanicCode::StorageEncodingError)); + return w.render(); + }); +} + std::string YulUtilFunctions::resizeDynamicArrayFunction(ArrayType const& _type) { solAssert(_type.location() == DataLocation::Storage, ""); diff --git a/libsolidity/codegen/YulUtilFunctions.h b/libsolidity/codegen/YulUtilFunctions.h index 9b1d59492..b8e7855e7 100644 --- a/libsolidity/codegen/YulUtilFunctions.h +++ b/libsolidity/codegen/YulUtilFunctions.h @@ -178,6 +178,12 @@ public: /// signature: (array) -> length std::string arrayLengthFunction(ArrayType const& _type); + /// @returns function name that extracts and returns byte array length from the value + /// stored at the slot. + /// Causes a Panic if the length encoding is wrong. + /// signature: (data) -> length + std::string extractByteArrayLengthFunction(); + /// @returns the name of a function that resizes a storage array /// signature: (array, newLen) std::string resizeDynamicArrayFunction(ArrayType const& _type); @@ -447,10 +453,6 @@ private: /// use exactly one variable to hold the value. std::string conversionFunctionSpecial(Type const& _from, Type const& _to); - /// @returns function name that extracts and returns byte array length - /// signature: (data) -> length - std::string extractByteArrayLengthFunction(); - /// @returns the name of a function that reduces the size of a storage byte array by one element /// signature: (byteArray) std::string storageByteArrayPopFunction(ArrayType const& _type); diff --git a/libsolutil/ErrorCodes.h b/libsolutil/ErrorCodes.h index d281b66b2..86129e3a4 100644 --- a/libsolutil/ErrorCodes.h +++ b/libsolutil/ErrorCodes.h @@ -29,6 +29,7 @@ enum class PanicCode UnderOverflow = 0x11, // arithmetic underflow or overflow DivisionByZero = 0x12, // division or modulo by zero EnumConversionError = 0x21, // enum conversion error + StorageEncodingError = 0x22, // invalid encoding in storage EmptyArrayPop = 0x31, // empty array pop ArrayOutOfBounds = 0x32, // array out of bounds access ResourceError = 0x41, // resource error (too large allocation or too large array) diff --git a/test/libsolidity/semanticTests/array/invalid_encoding_for_storage_byte_array.sol b/test/libsolidity/semanticTests/array/invalid_encoding_for_storage_byte_array.sol new file mode 100644 index 000000000..2fc66ece4 --- /dev/null +++ b/test/libsolidity/semanticTests/array/invalid_encoding_for_storage_byte_array.sol @@ -0,0 +1,92 @@ +contract C { + bytes public x = "abc"; + bytes public y; + function invalidateXShort() public { + assembly { sstore(x.slot, 64) } + delete y; + } + function invalidateXLong() public { + assembly { sstore(x.slot, 5) } + delete y; + } + function abiEncode() public view returns (bytes memory) { return x; } + function abiEncodePacked() public view returns (bytes memory) { return abi.encodePacked(x); } + function copyToMemory() public view returns (bytes memory m) { m = x; } + function indexAccess() public view returns (byte) { return x[0]; } + function assignTo() public { x = "def"; } + function assignToLong() public { x = "1234567890123456789012345678901234567"; } + function copyToStorage() public { y = x; } + function copyFromStorageShort() public { y = "abc"; x = y; } + function copyFromStorageLong() public { y = "1234567890123456789012345678901234567"; x = y; } + function arrayPop() public { x.pop(); } + function arrayPush() public { x.push("t"); } + function arrayPushEmpty() public { x.push(); } + function del() public { delete x; } +} +// ---- +// x() -> 0x20, 3, 0x6162630000000000000000000000000000000000000000000000000000000000 +// abiEncode() -> 0x20, 3, 0x6162630000000000000000000000000000000000000000000000000000000000 +// abiEncodePacked() -> 0x20, 3, 0x6162630000000000000000000000000000000000000000000000000000000000 +// copyToMemory() -> 0x20, 3, 0x6162630000000000000000000000000000000000000000000000000000000000 +// indexAccess() -> 0x6100000000000000000000000000000000000000000000000000000000000000 +// arrayPushEmpty() +// arrayPush() +// x() -> 0x20, 5, 0x6162630074000000000000000000000000000000000000000000000000000000 +// arrayPop() +// assignToLong() +// x() -> 0x20, 0x25, 0x3132333435363738393031323334353637383930313233343536373839303132, 0x3334353637000000000000000000000000000000000000000000000000000000 +// assignTo() +// x() -> 0x20, 3, 0x6465660000000000000000000000000000000000000000000000000000000000 +// copyFromStorageShort() +// x() -> 0x20, 3, 0x6162630000000000000000000000000000000000000000000000000000000000 +// copyFromStorageLong() +// x() -> 0x20, 0x25, 0x3132333435363738393031323334353637383930313233343536373839303132, 0x3334353637000000000000000000000000000000000000000000000000000000 +// copyToStorage() +// x() -> 0x20, 0x25, 0x3132333435363738393031323334353637383930313233343536373839303132, 0x3334353637000000000000000000000000000000000000000000000000000000 +// y() -> 0x20, 0x25, 0x3132333435363738393031323334353637383930313233343536373839303132, 0x3334353637000000000000000000000000000000000000000000000000000000 +// del() +// x() -> 0x20, 0x00 +// invalidateXLong() +// x() -> FAILURE, hex"4e487b71", 0x22 +// abiEncode() -> FAILURE, hex"4e487b71", 0x22 +// abiEncodePacked() -> FAILURE, hex"4e487b71", 0x22 +// copyToMemory() -> FAILURE, hex"4e487b71", 0x22 +// indexAccess() -> FAILURE, hex"4e487b71", 0x22 +// arrayPushEmpty() -> FAILURE, hex"4e487b71", 0x22 +// arrayPush() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// arrayPop() -> FAILURE, hex"4e487b71", 0x22 +// assignToLong() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// assignTo() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// copyFromStorageShort() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// copyFromStorageLong() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// copyToStorage() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// y() -> 0x20, 0x00 +// del() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// invalidateXShort() +// x() -> FAILURE, hex"4e487b71", 0x22 +// abiEncode() -> FAILURE, hex"4e487b71", 0x22 +// abiEncodePacked() -> FAILURE, hex"4e487b71", 0x22 +// copyToMemory() -> FAILURE, hex"4e487b71", 0x22 +// indexAccess() -> FAILURE, hex"4e487b71", 0x22 +// arrayPushEmpty() -> FAILURE, hex"4e487b71", 0x22 +// arrayPush() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// arrayPop() -> FAILURE, hex"4e487b71", 0x22 +// assignToLong() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// assignTo() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// copyFromStorageShort() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// copyFromStorageLong() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// copyToStorage() -> FAILURE, hex"4e487b71", 0x22 +// x() -> FAILURE, hex"4e487b71", 0x22 +// y() -> 0x20, 0x00