diff --git a/Changelog.md b/Changelog.md index 8709aa947..8d7ff006a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Important Bugfixes: * Add missing callvalue check to the creation code of a contract that does not define a constructor but has a base that does define a constructor. + * Disallow index range accesses for arrays with dynamically encoded base types. Language Features: diff --git a/docs/bugs.json b/docs/bugs.json index c251fa114..a32b27fb4 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,15 @@ [ + { + "name": "ArraySliceDynamicallyEncodedBaseType", + "summary": "Accessing array slices of arrays with dynamically encoded base types (e.g. multi-dimensional arrays) can result in invalid data being read.", + "description": "For arrays with dynamically sized base types, index range accesses that use a start expression that is non-zero will result in invalid array slices. Any index access to such array slices will result in data being read from incorrect calldata offsets. Array slices are only supported for dynamic calldata types and all problematic type require ABIEncoderV2 to be enabled.", + "introduced": "0.6.0", + "fixed": "0.6.8", + "severity": "very low", + "conditions": { + "ABIEncoderV2": true + } + }, { "name": "ImplicitConstructorCallvalueCheck", "summary": "The creation code of a contract that does not define a constructor but has a base that does define a constructor did not revert for calls with non-zero value.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 0398e4cd7..ac0c4b9f1 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1082,6 +1082,7 @@ }, "0.6.0": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", @@ -1091,6 +1092,7 @@ }, "0.6.1": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" @@ -1099,6 +1101,7 @@ }, "0.6.2": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" @@ -1107,6 +1110,7 @@ }, "0.6.3": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" @@ -1115,6 +1119,7 @@ }, "0.6.4": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" @@ -1123,6 +1128,7 @@ }, "0.6.5": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents" ], @@ -1130,12 +1136,14 @@ }, "0.6.6": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck" ], "released": "2020-04-09" }, "0.6.7": { "bugs": [ + "ArraySliceDynamicallyEncodedBaseType", "ImplicitConstructorCallvalueCheck" ], "released": "2020-05-04" diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 380879c73..ebb7bf6e6 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -2864,6 +2864,8 @@ bool TypeChecker::visit(IndexRangeAccess const& _access) if (arrayType->location() != DataLocation::CallData || !arrayType->isDynamicallySized()) m_errorReporter.typeError(1227_error, _access.location(), "Index range access is only supported for dynamic calldata arrays."); + else if (arrayType->baseType()->isDynamicallyEncoded()) + m_errorReporter.typeError(1878_error, _access.location(), "Index range access is not supported for arrays with dynamically encoded base types."); _access.annotation().type = TypeProvider::arraySlice(*arrayType); _access.annotation().isLValue = isLValue; _access.annotation().isPure = isPure; diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 156cee565..feae52caa 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -990,7 +990,8 @@ void CompilerUtils::convertType( solAssert(_targetType == typeOnStack.arrayType(), ""); solUnimplementedAssert( typeOnStack.arrayType().location() == DataLocation::CallData && - typeOnStack.arrayType().isDynamicallySized(), + typeOnStack.arrayType().isDynamicallySized() && + !typeOnStack.arrayType().baseType()->isDynamicallyEncoded(), "" ); break; diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 7b07cc48a..f05ffda8f 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -1769,7 +1769,12 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) case Type::Category::ArraySlice: { auto const& arrayType = dynamic_cast(baseType).arrayType(); - solAssert(arrayType.location() == DataLocation::CallData && arrayType.isDynamicallySized(), ""); + solAssert( + arrayType.location() == DataLocation::CallData && + arrayType.isDynamicallySized() && + !arrayType.baseType()->isDynamicallyEncoded(), + "" + ); solAssert(_indexAccess.indexExpression(), "Index expression expected."); acceptAndConvert(*_indexAccess.indexExpression(), *TypeProvider::uint256(), true); @@ -1852,7 +1857,12 @@ bool ExpressionCompiler::visit(IndexRangeAccess const& _indexAccess) arrayType = &sliceType->arrayType(); solAssert(arrayType, ""); - solUnimplementedAssert(arrayType->location() == DataLocation::CallData && arrayType->isDynamicallySized(), ""); + solUnimplementedAssert( + arrayType->location() == DataLocation::CallData && + arrayType->isDynamicallySized() && + !arrayType->baseType()->isDynamicallyEncoded(), + "" + ); if (_indexAccess.startExpression()) acceptAndConvert(*_indexAccess.startExpression(), *TypeProvider::uint256()); diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index f7bb31447..4fd681e1f 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -1599,6 +1599,7 @@ string YulUtilFunctions::conversionFunction(Type const& _from, Type const& _to) ArraySliceType const& fromType = dynamic_cast(_from); ArrayType const& targetType = dynamic_cast(_to); + solAssert(!fromType.arrayType().baseType()->isDynamicallyEncoded(), ""); solAssert( *fromType.arrayType().baseType() == *targetType.baseType(), "Converting arrays of different type is not possible" diff --git a/test/libsolidity/syntaxTests/parsing/array_range_nested.sol b/test/libsolidity/syntaxTests/parsing/array_range_nested.sol index 847d7596d..16fde7999 100644 --- a/test/libsolidity/syntaxTests/parsing/array_range_nested.sol +++ b/test/libsolidity/syntaxTests/parsing/array_range_nested.sol @@ -2,9 +2,8 @@ pragma experimental ABIEncoderV2; contract C { function f(uint256[][] calldata x) external pure { x[0][1:2]; - x[1:2][1:2]; - uint256 a = x[1:2][1:2][1:][3:][0][2]; - uint256 b = x[1:][3:4][1][1:][2:3][0]; + uint256 a = x[0][1:2][1:2][1:][3:][0]; + uint256 b = x[1][1:][3:4][1:][2:3][0]; a; b; } } diff --git a/test/libsolidity/syntaxTests/parsing/array_range_nested_invalid.sol b/test/libsolidity/syntaxTests/parsing/array_range_nested_invalid.sol new file mode 100644 index 000000000..bc53d3faf --- /dev/null +++ b/test/libsolidity/syntaxTests/parsing/array_range_nested_invalid.sol @@ -0,0 +1,14 @@ +pragma experimental ABIEncoderV2; +contract C { + function f(uint256[][] calldata x) external pure { + x[1:2]; + x[:]; + x[1:]; + x[:2]; + } +} +// ---- +// TypeError: (110-116): Index range access is not supported for arrays with dynamically encoded base types. +// TypeError: (126-130): Index range access is not supported for arrays with dynamically encoded base types. +// TypeError: (140-145): Index range access is not supported for arrays with dynamically encoded base types. +// TypeError: (155-160): Index range access is not supported for arrays with dynamically encoded base types.