From 5a3dbb0269b3ff6b443a3cb4ccfc4f00eaba26b4 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 13 Oct 2017 17:29:09 +0200 Subject: Cleanup and overflow checks for data pointers. --- libsolidity/codegen/ABIFunctions.cpp | 64 ++++++++++++++++++++++---------- libsolidity/codegen/ABIFunctions.h | 4 +- libsolidity/codegen/CompilerContext.cpp | 1 - libsolidity/codegen/ContractCompiler.cpp | 1 - 4 files changed, 47 insertions(+), 23 deletions(-) (limited to 'libsolidity/codegen') diff --git a/libsolidity/codegen/ABIFunctions.cpp b/libsolidity/codegen/ABIFunctions.cpp index c9a9ff57..6648be06 100644 --- a/libsolidity/codegen/ABIFunctions.cpp +++ b/libsolidity/codegen/ABIFunctions.cpp @@ -111,9 +111,9 @@ string ABIFunctions::tupleDecoder(TypePointers const& _types, bool _fromMemory) if (_fromMemory) functionName += "_fromMemory"; - return createFunction(functionName, [&]() { - solAssert(!_types.empty(), ""); + solAssert(!_types.empty(), ""); + return createFunction(functionName, [&]() { TypePointers decodingTypes; for (auto const& t: _types) decodingTypes.emplace_back(t->decodingType()); @@ -146,16 +146,22 @@ string ABIFunctions::tupleDecoder(TypePointers const& _types, bool _fromMemory) stackPos++; } bool dynamic = decodingTypes[i]->isDynamicallyEncoded(); - Whiskers elementTempl(R"( + Whiskers elementTempl( + dynamic ? + R"( { - let offset := )" + string( - dynamic ? - "(add(headStart, ))" : - "" - ) + R"( + let offset := (add(headStart, )) + switch gt(offset, 0xffffffffffffffff) case 1 { revert(0, 0) } := (add(headStart, offset), dataEnd) } - )"); + )" : + R"( + { + let offset := + := (add(headStart, offset), dataEnd) + } + )" + ); elementTempl("load", _fromMemory ? "mload" : "calldataload"); elementTempl("values", boost::algorithm::join(valueNamesLocal, ", ")); elementTempl("pos", to_string(headPos)); @@ -1053,6 +1059,10 @@ string ABIFunctions::abiEncodingFunctionFunctionType( string ABIFunctions::abiDecodingFunction(Type const& _type, bool _fromMemory, bool _forUseOnStack) { + // The decoding function has to perform bounds checks unless it decodes a value type. + // Conversely, bounds checks have to be performed before the decoding function + // of a value type is called. + TypePointer decodingType = _type.decodingType(); solAssert(decodingType, ""); @@ -1072,7 +1082,14 @@ string ABIFunctions::abiDecodingFunction(Type const& _type, bool _fromMemory, bo return abiDecodingFunctionStruct(*structType, _fromMemory); else if (auto const* functionType = dynamic_cast(decodingType.get())) return abiDecodingFunctionFunctionType(*functionType, _fromMemory, _forUseOnStack); + else + return abiDecodingFunctionValueType(_type, _fromMemory); +} +string ABIFunctions::abiDecodingFunctionValueType(const Type& _type, bool _fromMemory) +{ + TypePointer decodingType = _type.decodingType(); + solAssert(decodingType, ""); solAssert(decodingType->sizeOnStack() == 1, ""); solAssert(decodingType->isValueType(), ""); solAssert(decodingType->calldataEncodedSize() == 32, ""); @@ -1095,6 +1112,7 @@ string ABIFunctions::abiDecodingFunction(Type const& _type, bool _fromMemory, bo templ("cleanup", cleanupFunction(_type, true)); return templ.render(); }); + } string ABIFunctions::abiDecodingFunctionArray(ArrayType const& _type, bool _fromMemory) @@ -1116,6 +1134,7 @@ string ABIFunctions::abiDecodingFunctionArray(ArrayType const& _type, bool _from R"( // function (offset, end) -> array { + switch slt(add(offset, 0x1f), end) case 0 { revert(0, 0) } let length := array := ((length)) let dst := array @@ -1125,7 +1144,6 @@ string ABIFunctions::abiDecodingFunctionArray(ArrayType const& _type, bool _from for { let i := 0 } lt(i, length) { i := add(i, 1) } { let elementPos := - mstore(dst, (elementPos, end)) dst := add(dst, 0x20) src := add(src, ) @@ -1145,10 +1163,6 @@ string ABIFunctions::abiDecodingFunctionArray(ArrayType const& _type, bool _from if (dynamicBase) { templ("staticBoundsCheck", ""); - // The dynamic bounds check might not be needed (because we have an additional check - // one level deeper), but we keep it in just in case. This at least prevents - // the part one level deeper from reading the length from an out of bounds position. - templ("dynamicBoundsCheck", "switch gt(elementPos, end) case 1 { revert(0, 0) }"); templ("retrieveElementPos", "add(offset, " + load + "(src))"); templ("baseEncodedSize", "0x20"); } @@ -1156,7 +1170,6 @@ string ABIFunctions::abiDecodingFunctionArray(ArrayType const& _type, bool _from { string baseEncodedSize = toCompactHexWithPrefix(_type.baseType()->calldataEncodedSize()); templ("staticBoundsCheck", "switch gt(add(src, mul(length, " + baseEncodedSize + ")), end) case 1 { revert(0, 0) }"); - templ("dynamicBoundsCheck", ""); templ("retrieveElementPos", "src"); templ("baseEncodedSize", baseEncodedSize); } @@ -1184,6 +1197,7 @@ string ABIFunctions::abiDecodingFunctionCalldataArray(ArrayType const& _type) templ = R"( // function (offset, end) -> arrayPos, length { + switch slt(add(offset, 0x1f), end) case 0 { revert(0, 0) } length := calldataload(offset) switch gt(length, 0xffffffffffffffff) case 1 { revert(0, 0) } arrayPos := add(offset, 0x20) @@ -1221,6 +1235,7 @@ string ABIFunctions::abiDecodingFunctionByteArray(ArrayType const& _type, bool _ Whiskers templ( R"( function (offset, end) -> array { + switch slt(add(offset, 0x1f), end) case 0 { revert(0, 0) } let length := (offset) array := ((length)) mstore(array, length) @@ -1277,10 +1292,18 @@ string ABIFunctions::abiDecodingFunctionStruct(StructType const& _type, bool _fr auto decodingType = member.type->decodingType(); solAssert(decodingType, ""); bool dynamic = decodingType->isDynamicallyEncoded(); - Whiskers memberTempl(R"( - let offset := )" + string(dynamic ? "(add(headStart, ))" : "" ) + R"( - mstore(add(value, ), (add(headStart, offset), end)) - )"); + Whiskers memberTempl( + dynamic ? + R"( + let offset := (add(headStart, )) + switch gt(offset, 0xffffffffffffffff) case 1 { revert(0, 0) } + mstore(add(value, ), (add(headStart, offset), end)) + )" : + R"( + let offset := + mstore(add(value, ), (add(headStart, offset), end)) + )" + ); memberTempl("load", _fromMemory ? "mload" : "calldataload"); memberTempl("pos", to_string(headPos)); memberTempl("memoryOffset", toCompactHexWithPrefix(_type.memoryOffsetOfMember(member.name))); @@ -1596,7 +1619,8 @@ string ABIFunctions::allocationFunction() function (size) -> memPtr { memPtr := mload() let newFreePtr := add(memPtr, size) - switch lt(newFreePtr, memPtr) case 1 { revert(0, 0) } + // protect against overflow + switch or(gt(newFreePtr, 0xffffffffffffffff), lt(newFreePtr, memPtr)) case 1 { revert(0, 0) } mstore(, newFreePtr) } )") diff --git a/libsolidity/codegen/ABIFunctions.h b/libsolidity/codegen/ABIFunctions.h index 855b2a11..2b582e84 100644 --- a/libsolidity/codegen/ABIFunctions.h +++ b/libsolidity/codegen/ABIFunctions.h @@ -160,7 +160,7 @@ private: bool _fromStack ); - /// @returns the name of the ABI decodinf function for the given type + /// @returns the name of the ABI decoding function for the given type /// and queues the generation of the function to the requested functions. /// The caller has to ensure that no out of bounds access (at least to the static /// part) can happen inside this function. @@ -172,6 +172,8 @@ private: bool _forUseOnStack ); + /// Part of @a abiDecodingFunction for value types. + std::string abiDecodingFunctionValueType(Type const& _type, bool _fromMemory); /// Part of @a abiDecodingFunction for "regular" array types. std::string abiDecodingFunctionArray(ArrayType const& _type, bool _fromMemory); /// Part of @a abiDecodingFunction for calldata array types. diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 35d763f1..ce9c3b7f 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -318,7 +318,6 @@ void CompilerContext::appendInlineAssembly( ErrorList errors; ErrorReporter errorReporter(errors); -// cout << _assembly << endl; auto scanner = make_shared(CharStream(_assembly), "--CODEGEN--"); auto parserResult = assembly::Parser(errorReporter).parse(scanner); #ifdef SOL_OUTPUT_ASM diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 1ae44165..a81ba518 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -901,7 +901,6 @@ void ContractCompiler::appendMissingFunctions() } m_context.appendMissingLowLevelFunctions(); string abiFunctions = m_context.abiFunctions().requestedFunctions(); -// cout << abiFunctions << endl; if (!abiFunctions.empty()) m_context.appendInlineAssembly("{" + move(abiFunctions) + "}", {}, true); } -- cgit