Add stricter hex underscore rules

4 files changed, 67 insertions, 17 deletions

diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp
index 77492499..dc2e35e5 100644
--- a/libsolidity/analysis/SyntaxChecker.cpp
+++ b/libsolidity/analysis/SyntaxChecker.cpp
@@ -24,6 +24,9 @@
 #include <libsolidity/interface/Version.h>
 #include <boost/algorithm/cxx11/all_of.hpp>
 
+#include <boost/algorithm/string.hpp>
+#include <string>
+
 using namespace std;
 using namespace dev;
 using namespace dev::solidity;
@@ -183,6 +186,46 @@ bool SyntaxChecker::visit(Throw const& _throwStatement)
 	return true;
 }
 
+bool SyntaxChecker::visit(Literal const& _literal)
+{
+	if (!_literal.isHexNumber())
+		return true;
+	// We have a hex literal. Do underscore validation
+	solAssert(_literal.value().substr(0, 2) == "0x", "");
+	ASTString value = _literal.value().substr(2); // Skip the 0x
+	vector<ASTString> parts;
+	boost::split(parts, value, boost::is_any_of("_"));
+
+	if (parts.size() == 1) // no underscores
+		return true;
+	// Everything except first and last part must be 4 chars in length
+	for (size_t i = 1; i + 1 < parts.size(); ++i)
+	{
+		if (parts[i].size() != 4)
+			m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in hex literal. Found inner part with " + to_string(parts[i].size()) + " digits (has to be 4 digits).");
+	}
+
+	// Validate rightmost block
+	if (parts.back().size() == 4) // If ends with 4 digits, then no need to validate first block
+		return true;
+
+	// Validate leftmost block
+	// If first part is 4 digits then last part's length has to be even to avoid ambiguity over zero padding
+	if (parts.front().size() == 4)
+	{
+		if (parts.back().size() % 2 == 0)
+			return true;
+		m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in hex literal. If the first part has 4 digits, it is assumed to be a byte sequence instead of a number and thus the last part should have an even number of digits.");
+	}
+	else
+	{
+		// Both first and last part is invalid
+		m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in hex literal. First or last part must have 4 digits.");
+	}
+
+	return true;
+}
+
 bool SyntaxChecker::visit(UnaryOperation const& _operation)
 {
 	if (_operation.getOperator() == Token::Add)
diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h
index 28a0f66e..897df676 100644
--- a/libsolidity/analysis/SyntaxChecker.h
+++ b/libsolidity/analysis/SyntaxChecker.h
@@ -73,6 +73,7 @@ private:
 	virtual bool visit(VariableDeclarationStatement const& _statement) override;
 
 	virtual bool visit(StructDefinition const& _struct) override;
+	virtual bool visit(Literal const& _literal) override;
 
 	ErrorReporter& m_errorReporter;
 
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 3eccc6d4..73137ba9 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -39,6 +39,7 @@
 #include <boost/range/algorithm/copy.hpp>
 #include <boost/range/adaptor/sliced.hpp>
 #include <boost/range/adaptor/transformed.hpp>
+#include <boost/algorithm/string.hpp>
 
 #include <limits>
 
@@ -779,7 +780,9 @@ tuple<bool, rational> RationalNumberType::isValidLiteral(Literal const& _literal
 		if (boost::starts_with(_literal.value(), "0x"))
 		{
 			// process as hex
-			value = bigint(_literal.value());
+			ASTString valueString = _literal.value();
+			boost::erase_all(valueString, "_");// Remove underscore separators
+			value = bigint(valueString);
 		}
 		else if (expPoint != _literal.value().end())
 		{
diff --git a/libsolidity/parsing/Scanner.cpp b/libsolidity/parsing/Scanner.cpp
index 65189b19..c223779e 100644
--- a/libsolidity/parsing/Scanner.cpp
+++ b/libsolidity/parsing/Scanner.cpp
@@ -726,21 +726,26 @@ Token::Value Scanner::scanHexString()
 
 void Scanner::scanDecimalDigits()
 {
-	if (!isDecimalDigit(m_char)) // avoid underscore at beginning
-		return;
-	while (isDecimalDigit(m_char) || m_char == '_') 
+	// Parse for regex [:digit:]+(_[:digit:]+)*
+
+	do
 	{
+		if (!isDecimalDigit(m_char))
+			return;
+		while (isDecimalDigit(m_char))
+			addLiteralCharAndAdvance();
+
 		if (m_char == '_') 
 		{
 			advance();
-			if (!isDecimalDigit(m_char)) // avoid trailing underscore
+			if (!isDecimalDigit(m_char)) // Trailing underscore. Rollback and allow next step to flag it as illegal
 			{
 				rollback(1);
-				break;
+				return;
 			}
 		}
-		addLiteralCharAndAdvance();
 	}
+	while (isDecimalDigit(m_char));
 }
 
 Token::Value Scanner::scanNumber(char _charSeen)
@@ -768,19 +773,17 @@ Token::Value Scanner::scanNumber(char _charSeen)
 				addLiteralCharAndAdvance();
 				if (!isHexDigit(m_char))
 					return Token::Illegal; // we must have at least one hex digit after 'x'/'X'
-				while (isHexDigit(m_char) || m_char == '_') // same logic as scanDecimalDigits
+				char last = m_char;
+				while (isHexDigit(m_char) || m_char == '_') // Unlike decimal digits, we keep the underscores for later validation
 				{
-					if (m_char == '_') 
-					{
-						advance();
-						if (!isHexDigit(m_char)) // avoid trailing underscore
-						{
-							rollback(1);
-							break;
-						}
-					}
+					if (m_char == '_' && last == '_')
+						return Token::Illegal; // Double underscore
+
+					last = m_char;
 					addLiteralCharAndAdvance();
 				}
+				if (last == '_')
+					return Token::Illegal; // Trailing underscore
 			}
 			else if (isDecimalDigit(m_char))
 				// We do not allow octal numbers