Merge pull request #32 from LianaHus/sol_compiletime_check_for_out_of_bound_access_for_arrays

Sol compiletime check for out of bound index(integer constant) access for Ordinary arrays

4 files changed, 71 insertions, 36 deletions

diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp
index 674f33b7..daa84016 100644
--- a/libsolidity/AST.cpp
+++ b/libsolidity/AST.cpp
@@ -58,11 +58,11 @@ void ContractDefinition::checkTypeRequirements()
 	checkAbstractFunctions();
 	checkAbstractConstructors();
 
-	FunctionDefinition const* function = constructor();
-	if (function && !function->returnParameters().empty())
-		BOOST_THROW_EXCEPTION(
-			function->returnParameterList()->createTypeError("Non-empty \"returns\" directive for constructor.")
-		);
+	FunctionDefinition const* functionDefinition = constructor();
+	if (functionDefinition && !functionDefinition->returnParameters().empty())
+		BOOST_THROW_EXCEPTION(functionDefinition->returnParameterList()->createTypeError(
+			"Non-empty \"returns\" directive for constructor."
+		));
 
 	FunctionDefinition const* fallbackFunction = nullptr;
 	for (ASTPointer<FunctionDefinition> const& function: definedFunctions())
@@ -178,7 +178,9 @@ void ContractDefinition::checkDuplicateFunctions() const
 						errinfo_sourceLocation(overloads[j]->location()) <<
 						errinfo_comment("Function with same name and arguments defined twice.") <<
 						errinfo_secondarySourceLocation(SecondarySourceLocation().append(
-							"Other declaration is here:", overloads[i]->location()))
+							"Other declaration is here:", overloads[i]->location())
+						)
+
 					);
 	}
 }
@@ -767,8 +769,10 @@ void Return::checkTypeRequirements()
 	if (!m_returnParameters)
 		BOOST_THROW_EXCEPTION(createTypeError("Return arguments not allowed."));
 	if (m_returnParameters->parameters().size() != 1)
-		BOOST_THROW_EXCEPTION(createTypeError("Different number of arguments in return statement "
-											  "than in returns declaration."));
+		BOOST_THROW_EXCEPTION(createTypeError(
+			"Different number of arguments in return statement "
+			"than in returns declaration."
+		));
 	// this could later be changed such that the paramaters type is an anonymous struct type,
 	// but for now, we only allow one return parameter
 	m_expression->expectType(*m_returnParameters->parameters().front()->type());
@@ -795,10 +799,14 @@ void Assignment::checkTypeRequirements(TypePointers const*)
 		TypePointer resultType = m_type->binaryOperatorResult(Token::AssignmentToBinaryOp(m_assigmentOperator),
 															  m_rightHandSide->type());
 		if (!resultType || *resultType != *m_type)
-			BOOST_THROW_EXCEPTION(createTypeError("Operator " + string(Token::toString(m_assigmentOperator)) +
-												  " not compatible with types " +
-												  m_type->toString() + " and " +
-												  m_rightHandSide->type()->toString()));
+			BOOST_THROW_EXCEPTION(createTypeError(
+				"Operator " +
+				string(Token::toString(m_assigmentOperator)) +
+				" not compatible with types " +
+				m_type->toString() +
+				" and " +
+				m_rightHandSide->type()->toString()
+			));
 	}
 }
 
@@ -850,16 +858,13 @@ void BinaryOperation::checkTypeRequirements(TypePointers const*)
 	m_right->checkTypeRequirements(nullptr);
 	m_commonType = m_left->type()->binaryOperatorResult(m_operator, m_right->type());
 	if (!m_commonType)
-		BOOST_THROW_EXCEPTION(
-			createTypeError(
-				"Operator " +
-				string(Token::toString(m_operator)) +
-				" not compatible with types " +
-				m_left->type()->toString() +
-				" and " +
-				m_right->type()->toString()
-			)
-		);
+		BOOST_THROW_EXCEPTION(createTypeError(
+			"Operator " + string(Token::toString(m_operator)) +
+			" not compatible with types " +
+			m_left->type()->toString() +
+			" and " +
+			m_right->type()->toString()
+		));
 	m_type = Token::isCompareOp(m_operator) ? make_shared<BoolType>() : m_commonType;
 }
 
@@ -1040,7 +1045,8 @@ void NewExpression::checkTypeRequirements(TypePointers const*)
 		TypePointers{contractType},
 		strings(),
 		strings(),
-		FunctionType::Location::Creation);
+		FunctionType::Location::Creation
+	);
 }
 
 void MemberAccess::checkTypeRequirements(TypePointers const* _argumentTypes)
@@ -1069,19 +1075,25 @@ void MemberAccess::checkTypeRequirements(TypePointers const* _argumentTypes)
 		);
 		if (!storageType->members().membersByName(*m_memberName).empty())
 			BOOST_THROW_EXCEPTION(createTypeError(
-				"Member \"" + *m_memberName + "\" is not available in " +
+				"Member \"" +
+				*m_memberName +
+				"\" is not available in " +
 				type.toString() +
 				" outside of storage."
 			));
 		BOOST_THROW_EXCEPTION(createTypeError(
-			"Member \"" + *m_memberName + "\" not found or not visible "
-			"after argument-dependent lookup in " + type.toString()
+			"Member \"" +
+			*m_memberName +
+			"\" not found or not visible after argument-dependent lookup in " +
+			type.toString()
 		));
 	}
 	else if (possibleMembers.size() > 1)
 		BOOST_THROW_EXCEPTION(createTypeError(
-			"Member \"" + *m_memberName + "\" not unique "
-			"after argument-dependent lookup in " + type.toString()
+			"Member \"" +
+			*m_memberName +
+			"\" not unique after argument-dependent lookup in " +
+			type.toString()
 		));
 
 	m_referencedDeclaration = possibleMembers.front().declaration;
@@ -1114,10 +1126,12 @@ void IndexAccess::checkTypeRequirements(TypePointers const*)
 		if (type.isString())
 			BOOST_THROW_EXCEPTION(createTypeError("Index access for string is not possible."));
 		m_index->expectType(IntegerType(256));
-		if (type.isByteArray())
-			m_type = make_shared<FixedBytesType>(1);
-		else
-			m_type = type.baseType();
+
+		m_type = type.baseType();
+		if (auto integerType = dynamic_cast<IntegerConstantType const*>(m_index->type().get()))
+			if (!type.isDynamicallySized() && type.length() <= integerType->literalValue(nullptr))
+				BOOST_THROW_EXCEPTION(createTypeError("Out of bounds access."));
+
 		m_isLValue = type.location() != DataLocation::CallData;
 		break;
 	}
@@ -1143,7 +1157,8 @@ void IndexAccess::checkTypeRequirements(TypePointers const*)
 			if (!length)
 				BOOST_THROW_EXCEPTION(m_index->createTypeError("Integer constant expected."));
 			m_type = make_shared<TypeType>(make_shared<ArrayType>(
-				DataLocation::Memory, type.actualType(),
+				DataLocation::Memory,
+				type.actualType(),
 				length->literalValue(nullptr)
 			));
 		}
@@ -1151,7 +1166,10 @@ void IndexAccess::checkTypeRequirements(TypePointers const*)
 	}
 	default:
 		BOOST_THROW_EXCEPTION(m_base->createTypeError(
-			"Indexed expression has to be a type, mapping or array (is " + m_base->type()->toString() + ")"));
+			"Indexed expression has to be a type, mapping or array (is " +
+			m_base->type()->toString() +
+			")"
+		));
 	}
 }
 
diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp
index b22a78dc..1b9f2150 100644
--- a/libsolidity/ExpressionCompiler.cpp
+++ b/libsolidity/ExpressionCompiler.cpp
@@ -821,6 +821,7 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
 	_indexAccess.baseExpression().accept(*this);
 
 	Type const& baseType = *_indexAccess.baseExpression().type();
+
 	if (baseType.category() == Type::Category::Mapping)
 	{
 		// stack: storage_base_ref
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index c56845aa..2ba6ab85 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -1248,6 +1248,7 @@ BOOST_AUTO_TEST_CASE(convert_fixed_bytes_to_fixed_bytes_same_size)
 	compileAndRun(sourceCode);
 	BOOST_CHECK(callContractFunction("bytesToBytes(bytes4)", "abcd") == encodeArgs("abcd"));
 }
+
 // fixed bytes to uint conversion tests
 BOOST_AUTO_TEST_CASE(convert_fixed_bytes_to_uint_same_size)
 {
@@ -1300,6 +1301,7 @@ BOOST_AUTO_TEST_CASE(convert_fixed_bytes_to_uint_greater_size)
 	BOOST_CHECK(callContractFunction("bytesToUint(bytes4)", string("abcd")) ==
 		encodeArgs(u256("0x61626364")));
 }
+
 // uint fixed bytes conversion tests
 BOOST_AUTO_TEST_CASE(convert_uint_to_fixed_bytes_same_size)
 {
@@ -4188,7 +4190,8 @@ BOOST_AUTO_TEST_CASE(evm_exceptions_in_constructor_out_of_baund)
 			uint[3] arr;
 			function A()
 			{
-				test = arr[5];
+				uint index = 5;
+				test = arr[index];
 				++test;
 			}
 		}
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 5d174367..2a720494 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -2250,10 +2250,23 @@ BOOST_AUTO_TEST_CASE(creating_contract_within_the_contract)
 			function f() { var x = new Test(); }
 		}
 	)";
-
 	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError);
 }
 
+BOOST_AUTO_TEST_CASE(array_out_of_bound_access)
+{
+	char const* text = R"(
+		contract c {
+			uint[2] dataArray;
+			function set5th() returns (bool) {
+				dataArray[5] = 2;
+				return true;
+			}
+		}
+	)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }