Merge pull request #5242 from ethereum/someChecks

Some well-formedness checks for the Yul AST.

9 files changed, 95 insertions, 4 deletions

diff --git a/libdevcore/CommonData.cpp b/libdevcore/CommonData.cpp
index 6d7c74d7..fa1a5353 100644
--- a/libdevcore/CommonData.cpp
+++ b/libdevcore/CommonData.cpp
@@ -110,3 +110,26 @@ string dev::getChecksummedAddress(string const& _addr)
 	}
 	return ret;
 }
+
+bool dev::isValidHex(string const& _string)
+{
+	if (_string.substr(0, 2) != "0x")
+		return false;
+	if (_string.find_first_not_of("0123456789abcdefABCDEF", 2) != string::npos)
+		return false;
+	return true;
+}
+
+bool dev::isValidDecimal(string const& _string)
+{
+	if (_string.empty())
+		return false;
+	if (_string == "0")
+		return true;
+	// No leading zeros
+	if (_string.front() == '0')
+		return false;
+	if (_string.find_first_not_of("0123456789") != string::npos)
+		return false;
+	return true;
+}
diff --git a/libdevcore/CommonData.h b/libdevcore/CommonData.h
index f208c425..0782fabc 100644
--- a/libdevcore/CommonData.h
+++ b/libdevcore/CommonData.h
@@ -272,4 +272,7 @@ bool passesAddressChecksum(std::string const& _str, bool _strict);
 /// @param hex strings that look like an address
 std::string getChecksummedAddress(std::string const& _addr);
 
+bool isValidHex(std::string const& _string);
+bool isValidDecimal(std::string const& _string);
+
 }
diff --git a/libsolidity/inlineasm/AsmPrinter.cpp b/libsolidity/inlineasm/AsmPrinter.cpp
index 1a15d7eb..4b8c9538 100644
--- a/libsolidity/inlineasm/AsmPrinter.cpp
+++ b/libsolidity/inlineasm/AsmPrinter.cpp
@@ -24,6 +24,8 @@
 #include <libsolidity/inlineasm/AsmData.h>
 #include <libsolidity/interface/Exceptions.h>
 
+#include <libdevcore/CommonData.h>
+
 #include <boost/algorithm/string.hpp>
 #include <boost/algorithm/string/replace.hpp>
 #include <boost/range/adaptor/transformed.hpp>
@@ -41,6 +43,7 @@ using namespace dev::solidity::assembly;
 string AsmPrinter::operator()(assembly::Instruction const& _instruction)
 {
 	solAssert(!m_yul, "");
+	solAssert(isValidInstruction(_instruction.instruction), "Invalid instruction");
 	return boost::to_lower_copy(instructionInfo(_instruction.instruction).name);
 }
 
@@ -49,8 +52,10 @@ string AsmPrinter::operator()(assembly::Literal const& _literal)
 	switch (_literal.kind)
 	{
 	case LiteralKind::Number:
+		solAssert(isValidDecimal(_literal.value) || isValidHex(_literal.value), "Invalid number literal");
 		return _literal.value + appendTypeName(_literal.type);
 	case LiteralKind::Boolean:
+		solAssert(_literal.value == "true" || _literal.value == "false", "Invalid bool literal.");
 		return ((_literal.value == "true") ? "true" : "false") + appendTypeName(_literal.type);
 	case LiteralKind::String:
 		break;
@@ -87,18 +92,20 @@ string AsmPrinter::operator()(assembly::Literal const& _literal)
 
 string AsmPrinter::operator()(assembly::Identifier const& _identifier)
 {
+	solAssert(!_identifier.name.empty(), "Invalid identifier.");
 	return _identifier.name;
 }
 
 string AsmPrinter::operator()(assembly::FunctionalInstruction const& _functionalInstruction)
 {
 	solAssert(!m_yul, "");
+	solAssert(isValidInstruction(_functionalInstruction.instruction), "Invalid instruction");
 	return
 		boost::to_lower_copy(instructionInfo(_functionalInstruction.instruction).name) +
 		"(" +
 		boost::algorithm::join(
 			_functionalInstruction.arguments | boost::adaptors::transformed(boost::apply_visitor(*this)),
-			", " ) +
+			", ") +
 		")";
 }
 
@@ -110,12 +117,14 @@ string AsmPrinter::operator()(ExpressionStatement const& _statement)
 string AsmPrinter::operator()(assembly::Label const& _label)
 {
 	solAssert(!m_yul, "");
+	solAssert(!_label.name.empty(), "Invalid label.");
 	return _label.name + ":";
 }
 
 string AsmPrinter::operator()(assembly::StackAssignment const& _assignment)
 {
 	solAssert(!m_yul, "");
+	solAssert(!_assignment.variableName.name.empty(), "Invalid variable name.");
 	return "=: " + (*this)(_assignment.variableName);
 }
 
@@ -133,7 +142,7 @@ string AsmPrinter::operator()(assembly::VariableDeclaration const& _variableDecl
 	string out = "let ";
 	out += boost::algorithm::join(
 		_variableDeclaration.variables | boost::adaptors::transformed(
-			[this](TypedName variable) { return variable.name + appendTypeName(variable.type); }
+			[this](TypedName argument) { return formatTypedName(argument); }
 		),
 		", "
 	);
@@ -147,10 +156,11 @@ string AsmPrinter::operator()(assembly::VariableDeclaration const& _variableDecl
 
 string AsmPrinter::operator()(assembly::FunctionDefinition const& _functionDefinition)
 {
+	solAssert(!_functionDefinition.name.empty(), "Invalid function name.");
 	string out = "function " + _functionDefinition.name + "(";
 	out += boost::algorithm::join(
 		_functionDefinition.parameters | boost::adaptors::transformed(
-			[this](TypedName argument) { return argument.name + appendTypeName(argument.type); }
+			[this](TypedName argument) { return formatTypedName(argument); }
 		),
 		", "
 	);
@@ -160,7 +170,7 @@ string AsmPrinter::operator()(assembly::FunctionDefinition const& _functionDefin
 		out += " -> ";
 		out += boost::algorithm::join(
 			_functionDefinition.returnVariables | boost::adaptors::transformed(
-				[this](TypedName argument) { return argument.name + appendTypeName(argument.type); }
+				[this](TypedName argument) { return formatTypedName(argument); }
 			),
 			", "
 		);
@@ -181,11 +191,13 @@ string AsmPrinter::operator()(assembly::FunctionCall const& _functionCall)
 
 string AsmPrinter::operator()(If const& _if)
 {
+	solAssert(_if.condition, "Invalid if condition.");
 	return "if " + boost::apply_visitor(*this, *_if.condition) + "\n" + (*this)(_if.body);
 }
 
 string AsmPrinter::operator()(Switch const& _switch)
 {
+	solAssert(_switch.expression, "Invalid expression pointer.");
 	string out = "switch " + boost::apply_visitor(*this, *_switch.expression);
 	for (auto const& _case: _switch.cases)
 	{
@@ -200,6 +212,7 @@ string AsmPrinter::operator()(Switch const& _switch)
 
 string AsmPrinter::operator()(assembly::ForLoop const& _forLoop)
 {
+	solAssert(_forLoop.condition, "Invalid for loop condition.");
 	string out = "for ";
 	out += (*this)(_forLoop.pre);
 	out += "\n";
@@ -223,6 +236,12 @@ string AsmPrinter::operator()(Block const& _block)
 	return "{\n    " + body + "\n}";
 }
 
+string AsmPrinter::formatTypedName(TypedName _variable) const
+{
+	solAssert(!_variable.name.empty(), "Invalid variable name.");
+	return _variable.name + appendTypeName(_variable.type);
+}
+
 string AsmPrinter::appendTypeName(std::string const& _type) const
 {
 	if (m_yul)
diff --git a/libsolidity/inlineasm/AsmPrinter.h b/libsolidity/inlineasm/AsmPrinter.h
index 9f2b842a..971822bf 100644
--- a/libsolidity/inlineasm/AsmPrinter.h
+++ b/libsolidity/inlineasm/AsmPrinter.h
@@ -55,6 +55,7 @@ public:
 	std::string operator()(assembly::Block const& _block);
 
 private:
+	std::string formatTypedName(TypedName _variable) const;
 	std::string appendTypeName(std::string const& _type) const;
 
 	bool m_yul = false;
diff --git a/libyul/optimiser/SimplificationRules.cpp b/libyul/optimiser/SimplificationRules.cpp
index 4d0468c7..aca943b0 100644
--- a/libyul/optimiser/SimplificationRules.cpp
+++ b/libyul/optimiser/SimplificationRules.cpp
@@ -209,6 +209,7 @@ u256 Pattern::d() const
 {
 	Literal const& literal = boost::get<Literal>(matchGroupValue());
 	assertThrow(literal.kind == assembly::LiteralKind::Number, OptimizerException, "");
+	assertThrow(isValidDecimal(literal.value) || isValidHex(literal.value), OptimizerException, "");
 	return u256(literal.value);
 }
 
diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/empty_fun_arg.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/empty_fun_arg.sol
new file mode 100644
index 00000000..e05277de
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid/empty_fun_arg.sol
@@ -0,0 +1,13 @@
+contract C {
+  function f() public pure {
+    assembly {
+      function f(a, b) {}
+      f()
+      f(1,)
+      f(,1)
+    }
+  }
+}
+// ----
+// ParserError: (113-114): Literal, identifier or instruction expected.
+// ParserError: (113-114): Expected primary expression.
diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/empty_function_name.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/empty_function_name.sol
new file mode 100644
index 00000000..17995b09
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid/empty_function_name.sol
@@ -0,0 +1,10 @@
+contract C {
+  function f() public pure {
+    assembly {
+      function (a, b) {}
+    }
+  }
+}
+// ----
+// ParserError: (72-73): Expected identifier but got '('
+// ParserError: (79-80): Expected ';' but got '{'
diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/invalid_number.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/invalid_number.sol
new file mode 100644
index 00000000..715913de
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid/invalid_number.sol
@@ -0,0 +1,10 @@
+contract C {
+  function f() public pure {
+    assembly {
+      let x := 0100
+    }
+  }
+}
+// ----
+// ParserError: (72-73): Literal, identifier or instruction expected.
+// ParserError: (72-73): Expected primary expression.
diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/missing_variable_in_assign.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/missing_variable_in_assign.sol
new file mode 100644
index 00000000..c8984333
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid/missing_variable_in_assign.sol
@@ -0,0 +1,11 @@
+contract C {
+  function f() public pure {
+    assembly {
+      let x := mload(0)
+      := 1
+    }
+  }
+}
+// ----
+// ParserError: (87-88): Literal, identifier or instruction expected.
+// ParserError: (87-88): Expected primary expression.