From 38abadf50200122c3f6f064361111eeee8159205 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 15 Oct 2020 11:44:53 +0200 Subject: [PATCH] Fix empty array copy bug. --- Changelog.md | 4 + docs/bugs.json | 7 ++ docs/bugs_by_version.json | 83 ++++++++++++++++++- libsolidity/codegen/ArrayUtils.cpp | 9 ++ .../array/copying/empty_bytes_copy.sol | 28 +++++++ 5 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/semanticTests/array/copying/empty_bytes_copy.sol diff --git a/Changelog.md b/Changelog.md index ba746da82..407bd342e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,9 @@ ### 0.7.4 (unreleased) +Important Bugfixes: + * Code Generator: Fix data corruption bug when copying empty byte arrays from memory or calldata to storage. + + Language Features: * Constants can be defined at file level. diff --git a/docs/bugs.json b/docs/bugs.json index 357f50513..0f3be018a 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,11 @@ [ + { + "name": "EmptyByteArrayCopy", + "summary": "Copying an empty byte array (or string) from memory or calldata to storage can result in data corruption if the target array's length is increased subsequently without storing new data.", + "description": "The routine that copies byte arrays from memory or calldata to storage stores unrelated data from after the source array in the storage slot if the source array is empty. If the storage array's length is subsequently increased either by using ``.push()`` or by assigning to its ``.length`` attribute (only before 0.6.0), the newly created byte array elements will not be zero-initialized, but contain the unrelated data. You are not affected if you do not assign to ``.length`` and do not use ``.push()`` on byte arrays, or only use ``.push()`` or manually initialize the new elements.", + "fixed": "0.7.4", + "severity": "medium" + }, { "name": "DynamicArrayCleanup", "summary": "When assigning a dynamically-sized array with types of size at most 16 bytes in storage causing the assigned array to shrink, some parts of deleted slots were not zeroed out.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 40e7b4f3a..3f7c334ed 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1,6 +1,7 @@ { "0.1.0": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ExpExponentCleanup", "ZeroFunctionSelector", @@ -20,6 +21,7 @@ }, "0.1.1": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ExpExponentCleanup", "ZeroFunctionSelector", @@ -39,6 +41,7 @@ }, "0.1.2": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ExpExponentCleanup", "ZeroFunctionSelector", @@ -58,6 +61,7 @@ }, "0.1.3": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ExpExponentCleanup", "ZeroFunctionSelector", @@ -77,6 +81,7 @@ }, "0.1.4": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ExpExponentCleanup", "NestedArrayFunctionCallDecoder", @@ -97,6 +102,7 @@ }, "0.1.5": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ExpExponentCleanup", "NestedArrayFunctionCallDecoder", @@ -117,6 +123,7 @@ }, "0.1.6": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "ExpExponentCleanup", @@ -139,6 +146,7 @@ }, "0.1.7": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "ExpExponentCleanup", @@ -161,6 +169,7 @@ }, "0.2.0": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -184,6 +193,7 @@ }, "0.2.1": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -207,6 +217,7 @@ }, "0.2.2": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -230,6 +241,7 @@ }, "0.3.0": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -255,6 +267,7 @@ }, "0.3.1": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -279,6 +292,7 @@ }, "0.3.2": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -303,6 +317,7 @@ }, "0.3.3": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -326,6 +341,7 @@ }, "0.3.4": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -349,6 +365,7 @@ }, "0.3.5": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -372,6 +389,7 @@ }, "0.3.6": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -393,6 +411,7 @@ }, "0.4.0": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -414,6 +433,7 @@ }, "0.4.1": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -435,6 +455,7 @@ }, "0.4.10": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -455,6 +476,7 @@ }, "0.4.11": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -474,6 +496,7 @@ }, "0.4.12": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -492,6 +515,7 @@ }, "0.4.13": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -510,6 +534,7 @@ }, "0.4.14": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -527,6 +552,7 @@ }, "0.4.15": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -543,6 +569,7 @@ }, "0.4.16": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -561,6 +588,7 @@ }, "0.4.17": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -580,6 +608,7 @@ }, "0.4.18": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -598,6 +627,7 @@ }, "0.4.19": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -617,6 +647,7 @@ }, "0.4.2": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -637,6 +668,7 @@ }, "0.4.20": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -656,6 +688,7 @@ }, "0.4.21": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -675,6 +708,7 @@ }, "0.4.22": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -694,6 +728,7 @@ }, "0.4.23": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -712,6 +747,7 @@ }, "0.4.24": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -730,6 +766,7 @@ }, "0.4.25": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -746,6 +783,7 @@ }, "0.4.26": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -759,6 +797,7 @@ }, "0.4.3": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -778,6 +817,7 @@ }, "0.4.4": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -796,6 +836,7 @@ }, "0.4.5": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -817,6 +858,7 @@ }, "0.4.6": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -837,6 +879,7 @@ }, "0.4.7": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -857,6 +900,7 @@ }, "0.4.8": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -877,6 +921,7 @@ }, "0.4.9": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -897,6 +942,7 @@ }, "0.5.0": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -913,6 +959,7 @@ }, "0.5.1": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -929,6 +976,7 @@ }, "0.5.10": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -941,6 +989,7 @@ }, "0.5.11": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -952,6 +1001,7 @@ }, "0.5.12": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -963,6 +1013,7 @@ }, "0.5.13": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -974,6 +1025,7 @@ }, "0.5.14": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ImplicitConstructorCallvalueCheck", @@ -987,6 +1039,7 @@ }, "0.5.15": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ImplicitConstructorCallvalueCheck", @@ -999,6 +1052,7 @@ }, "0.5.16": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ImplicitConstructorCallvalueCheck", @@ -1010,6 +1064,7 @@ }, "0.5.17": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ImplicitConstructorCallvalueCheck", @@ -1020,6 +1075,7 @@ }, "0.5.2": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1036,6 +1092,7 @@ }, "0.5.3": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1052,6 +1109,7 @@ }, "0.5.4": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1068,6 +1126,7 @@ }, "0.5.5": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1086,6 +1145,7 @@ }, "0.5.6": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1104,6 +1164,7 @@ }, "0.5.7": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1120,6 +1181,7 @@ }, "0.5.8": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1135,6 +1197,7 @@ }, "0.5.9": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", @@ -1149,6 +1212,7 @@ }, "0.6.0": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1161,6 +1225,7 @@ }, "0.6.1": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1172,24 +1237,28 @@ }, "0.6.10": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup" ], "released": "2020-06-11" }, "0.6.11": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup" ], "released": "2020-07-07" }, "0.6.12": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup" ], "released": "2020-07-22" }, "0.6.2": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1201,6 +1270,7 @@ }, "0.6.3": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1212,6 +1282,7 @@ }, "0.6.4": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1223,6 +1294,7 @@ }, "0.6.5": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1233,6 +1305,7 @@ }, "0.6.6": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1242,6 +1315,7 @@ }, "0.6.7": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "MissingEscapingInFormatting", "ArraySliceDynamicallyEncodedBaseType", @@ -1251,12 +1325,14 @@ }, "0.6.8": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup" ], "released": "2020-05-14" }, "0.6.9": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "UsingForCalldata" ], @@ -1264,12 +1340,14 @@ }, "0.7.0": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup" ], "released": "2020-07-28" }, "0.7.1": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup", "FreeFunctionRedefinition" ], @@ -1277,12 +1355,15 @@ }, "0.7.2": { "bugs": [ + "EmptyByteArrayCopy", "DynamicArrayCleanup" ], "released": "2020-09-28" }, "0.7.3": { - "bugs": [], + "bugs": [ + "EmptyByteArrayCopy" + ], "released": "2020-10-07" } } \ No newline at end of file diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp index 1ae76bb45..14e302bc9 100644 --- a/libsolidity/codegen/ArrayUtils.cpp +++ b/libsolidity/codegen/ArrayUtils.cpp @@ -125,6 +125,15 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // special case for short byte arrays: Store them together with their length. if (_targetType.isByteArray()) { + // stack: target_ref target_data_end source_length target_data_pos source_ref + _context << Instruction::DUP3; + evmasm::AssemblyItem nonEmptyByteArray = _context.appendConditionalJump(); + // Empty source, just zero out the main slot. + _context << u256(0) << Instruction::DUP6 << Instruction::SSTORE; + _context.appendJumpTo(copyLoopEndWithoutByteOffset); + + _context << nonEmptyByteArray; + // Non-empty source. // stack: target_ref target_data_end source_length target_data_pos source_ref _context << Instruction::DUP3 << u256(31) << Instruction::LT; evmasm::AssemblyItem longByteArray = _context.appendConditionalJump(); diff --git a/test/libsolidity/semanticTests/array/copying/empty_bytes_copy.sol b/test/libsolidity/semanticTests/array/copying/empty_bytes_copy.sol new file mode 100644 index 000000000..91b6fd1a0 --- /dev/null +++ b/test/libsolidity/semanticTests/array/copying/empty_bytes_copy.sol @@ -0,0 +1,28 @@ +contract C { + bytes data; + bytes otherData; + function fromMemory() public returns (byte) { + bytes memory t; + uint[2] memory x; + x[0] = type(uint).max; + data = t; + data.push(); + return data[0]; + } + function fromCalldata(bytes calldata x) public returns (byte) { + data = x; + data.push(); + return data[0]; + } + function fromStorage() public returns (byte) { + // zero-length but dirty higher order bits + assembly { sstore(otherData.slot, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff00) } + data = otherData; + data.push(); + return data[0]; + } +} +// ---- +// fromMemory() -> 0x00 +// fromCalldata(bytes): 0x40, 0x60, 0x00, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff -> 0x00 +// fromStorage() -> 0x00