Refactor array decoding.

libsolidity/codegen/ABIFunctions.cpp

@@ -1182,12 +1182,28 @@ string ABIFunctions::abiDecodingFunctionArrayAvailableLength(ArrayType const& _t
 			function <functionName>(offset, length, end) -> array {
 				array := <allocate>(<allocationSize>(length))
 				let dst := array
-				<storeLength>
+				<?dynamic>
+					mstore(array, length)
+					dst := add(array, 0x20)
+				</dynamic>
 				let src := offset
-				<staticBoundsCheck>
+				<?dynamicBase>
+					// TODO add check that we can actually load from all
+					// offset pointers, i.e. as below, but with stride being 0x20.
+				<!dynamicBase>
+					if gt(add(src, mul(length, <stride>)), end) {
+						<revertInvalidStride>
+					}
+				</dynamicBase>
 				for { let i := 0 } lt(i, length) { i := add(i, 1) }
 				{
-					let elementPos := <retrieveElementPos>
+					<?dynamicBase>
+						let innerOffset := <load>(src)
+						// TODO add overflow check
+						let elementPos := add(offset, innerOffset)
+					<!dynamicBase>
+						let elementPos := src
+					</dynamicBase>
 					mstore(dst, <decodingFun>(elementPos, end))
 					dst := add(dst, 0x20)
 					src := add(src, <stride>)
@@ -1198,28 +1214,15 @@ string ABIFunctions::abiDecodingFunctionArrayAvailableLength(ArrayType const& _t
 		templ("readableTypeName", _type.toString(true));
 		templ("allocate", m_utils.allocationFunction());
 		templ("allocationSize", m_utils.arrayAllocationSizeFunction(_type));
-		string calldataStride = toCompactHexWithPrefix(_type.calldataStride());
-		templ("stride", calldataStride);
-		if (_type.isDynamicallySized())
-			templ("storeLength", "mstore(array, length) dst := add(array, 0x20)");
-		else
-			templ("storeLength", "");
-		if (_type.baseType()->isDynamicallyEncoded())
-		{
-			templ("staticBoundsCheck", "");
-			string load = _fromMemory ? "mload" : "calldataload";
-			templ("retrieveElementPos", "add(offset, " + load + "(src))");
-		}
-		else
-		{
-			templ("staticBoundsCheck", "if gt(add(src, mul(length, " +
-				calldataStride +
-				")), end) { " +
-				revertReasonIfDebug("ABI decoding: invalid calldata array stride") +
-				" }"
+		templ("stride", toCompactHexWithPrefix(_type.calldataStride()));
+		templ("dynamic", _type.isDynamicallySized());
+		templ("load", _fromMemory ? "mload" : "calldataload");
+		templ("dynamicBase", _type.baseType()->isDynamicallyEncoded());
+		if (!_type.baseType()->isDynamicallyEncoded())
+			templ(
+				"revertInvalidStride",
+				revertReasonIfDebug("ABI decoding: invalid calldata array stride")
 			);
-			templ("retrieveElementPos", "src");
-		}
 		templ("decodingFun", abiDecodingFunction(*_type.baseType(), _fromMemory, false));
 		return templ.render();
 	});

test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding.sol

@@ -0,0 +1,29 @@
+pragma abicoder v2;
+contract Test {
+	struct MemoryUint {
+		uint field;
+	}
+	function test() public pure returns (uint) {
+		uint[] memory before = new uint[](1); // at offset 0x80
+		// Two problems here: The first offset is zero, the second offset is missing.
+		bytes memory corrupt = abi.encode(uint(32), // offset to "tuple"
+										  uint(0)); // bogus first element
+		/*
+		  At this point the free pointer is 0x80 + 64 (size of before) + 32 (length field of corrupt) + 64 (two encoded words)
+
+		  Now let's put random junk into memory immediately after the bogus first element. Our goal is to overflow the read pointer to point to before.
+		  The value read out at this point will be added to beginning of the encoded tuple, AKA corrupt + 64. We need then to write x where:
+		  x + 0x80 + 64 (before) + 32 (length of corrupt) + 32 (first word of corrupt) = 0x80 (mod 2^256)
+		  that is MAX_UINT - 128
+		*/
+		MemoryUint memory afterCorrupt;
+		afterCorrupt.field = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80);
+		before[0] = 123456;
+		uint[][2] memory decoded = abi.decode(corrupt, (uint[][2]));
+		return decoded[1][0];
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// test() -> 0x01e240

test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_2.sol

@@ -0,0 +1,30 @@
+pragma abicoder v2;
+contract Test {
+	struct MemoryTuple {
+		uint field1;
+		uint field2;
+	}
+	function withinArray() public pure returns (uint) {
+		uint[] memory before = new uint[](1);
+		bytes memory corrupt = abi.encode(uint(32),
+										  uint(2));
+		MemoryTuple memory afterCorrupt;
+		before[0] = 123456;
+		/*
+		  As above, but in this case we are adding to:
+		  0x80 + 64 (before) + 32 (length of corrupt) + 32 (offset) + 32 (field pointer)
+		  giving MAX_UINT - 96
+		*/
+		afterCorrupt.field1 = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff60);
+		afterCorrupt.field2 = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff60);
+		uint[][] memory decoded = abi.decode(corrupt, (uint[][]));
+		/*
+		  Will return 123456 * 2, AKA before has been copied twice
+		 */
+		return decoded[0][0] + decoded[1][0];
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// withinArray() -> 0x03c480

test/libsolidity/semanticTests/abiencodedecode/offset_overflow_in_array_decoding_3.sol

@@ -0,0 +1,23 @@
+pragma abicoder v2;
+contract Test {
+	struct MemoryUint {
+		uint field;
+	}
+	function test() public pure returns (uint) {
+		uint[] memory before = new uint[](1); // at offset 0x80
+		bytes memory corrupt = abi.encode(
+            uint(32),
+            uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80),
+            uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80)
+        );
+		MemoryUint memory afterCorrupt;
+		afterCorrupt.field = uint(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff80);
+		before[0] = 123456;
+		uint[][2] memory decoded = abi.decode(corrupt, (uint[][2]));
+		return decoded[1][0];
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// test() -> 0x01e240