Merge pull request #8874 from ethereum/formattingBug

Escape backslashes when formatting

Changelog.md

@@ -3,6 +3,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.
+ * Code Generator: Fixed that string literals containing backslash characters could cause incorrect code to be generated when passed directly to function calls or encoding functions when ABIEncoderV2 is active.
 
 
 Language Features:

docs/bugs.json

@@ -1,4 +1,15 @@
 [
+    {
+        "name": "MissingEscapingInFormatting",
+        "summary": "String literals containing double backslash characters passed directly to external or encoding function calls can lead to a different string being used when ABIEncoderV2 is enabled.",
+        "description": "When ABIEncoderV2 is enabled, string literals passed directly to encoding functions or external function calls are stored as strings in the intemediate code. Characters outside the printable range are handled correctly, but backslashes are not escaped in this procedure. This leads to double backslashes being reduced to single backslashes and consequently re-interpreted as escapes potentially resulting in a different string being encoded.",
+        "introduced": "0.5.14",
+        "fixed": "0.6.8",
+        "severity": "very low",
+        "conditions": {
+            "ABIEncoderV2": true
+        }
+    },
     {
         "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.",

docs/bugs_by_version.json

@@ -923,6 +923,7 @@
     },
     "0.5.14": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
             "MemoryArrayCreationOverflow",
@@ -934,6 +935,7 @@
     },
     "0.5.15": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
             "MemoryArrayCreationOverflow",
@@ -944,6 +946,7 @@
     },
     "0.5.16": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
             "MemoryArrayCreationOverflow",
@@ -953,6 +956,7 @@
     },
     "0.5.17": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
             "MemoryArrayCreationOverflow"
@@ -1082,6 +1086,7 @@
     },
     "0.6.0": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
@@ -1092,6 +1097,7 @@
     },
     "0.6.1": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
@@ -1101,6 +1107,7 @@
     },
     "0.6.2": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
@@ -1110,6 +1117,7 @@
     },
     "0.6.3": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
@@ -1119,6 +1127,7 @@
     },
     "0.6.4": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents",
@@ -1128,6 +1137,7 @@
     },
     "0.6.5": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck",
             "TupleAssignmentMultiStackSlotComponents"
@@ -1136,6 +1146,7 @@
     },
     "0.6.6": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck"
         ],
@@ -1143,6 +1154,7 @@
     },
     "0.6.7": {
         "bugs": [
+            "MissingEscapingInFormatting",
             "ArraySliceDynamicallyEncodedBaseType",
             "ImplicitConstructorCallvalueCheck"
         ],

libsolutil/CommonData.cpp

@@ -188,5 +188,39 @@ string solidity::util::formatAsStringOrNumber(string const& _value)
 		if (c <= 0x1f || c >= 0x7f || c == '"')
 			return "0x" + h256(_value, h256::AlignLeft).hex();
 
-	return "\"" + _value + "\"";
+	return escapeAndQuoteString(_value);
+}
+
+
+string solidity::util::escapeAndQuoteString(string const& _input)
+{
+	string out;
+
+	for (char c: _input)
+		if (c == '\\')
+			out += "\\\\";
+		else if (c == '"')
+			out += "\\\"";
+		else if (c == '\b')
+			out += "\\b";
+		else if (c == '\f')
+			out += "\\f";
+		else if (c == '\n')
+			out += "\\n";
+		else if (c == '\r')
+			out += "\\r";
+		else if (c == '\t')
+			out += "\\t";
+		else if (c == '\v')
+			out += "\\v";
+		else if (!isprint(c, locale::classic()))
+		{
+			ostringstream o;
+			o << "\\x" << std::hex << setfill('0') << setw(2) << (unsigned)(unsigned char)(c);
+			out += o.str();
+		}
+		else
+			out += c;
+
+	return "\"" + std::move(out) + "\"";
 }

libsolutil/CommonData.h

@@ -460,6 +460,10 @@ bool isValidDecimal(std::string const& _string);
 /// _value cannot be longer than 32 bytes.
 std::string formatAsStringOrNumber(std::string const& _value);
 
+/// @returns a string with the usual backslash-escapes for non-ASCII
+/// characters and surrounded by '"'-characters.
+std::string escapeAndQuoteString(std::string const& _input);
+
 template<typename Container, typename Compare>
 bool containerEqual(Container const& _lhs, Container const& _rhs, Compare&& _compare)
 {

libyul/AsmPrinter.cpp

@@ -55,33 +55,7 @@ string AsmPrinter::operator()(Literal const& _literal) const
 		break;
 	}
 
-	string out;
-	for (char c: _literal.value.str())
-		if (c == '\\')
-			out += "\\\\";
-		else if (c == '"')
-			out += "\\\"";
-		else if (c == '\b')
-			out += "\\b";
-		else if (c == '\f')
-			out += "\\f";
-		else if (c == '\n')
-			out += "\\n";
-		else if (c == '\r')
-			out += "\\r";
-		else if (c == '\t')
-			out += "\\t";
-		else if (c == '\v')
-			out += "\\v";
-		else if (!isprint(c, locale::classic()))
-		{
-			ostringstream o;
-			o << std::hex << setfill('0') << setw(2) << (unsigned)(unsigned char)(c);
-			out += "\\x" + o.str();
-		}
-		else
-			out += c;
-	return "\"" + out + "\"" + appendTypeName(_literal.type);
+	return escapeAndQuoteString(_literal.value.str()) + appendTypeName(_literal.type);
 }
 
 string AsmPrinter::operator()(Identifier const& _identifier) const

test/libsolidity/semanticTests/literals/escape.sol

@@ -0,0 +1,13 @@
+pragma experimental ABIEncoderV2;
+
+contract C
+{
+	function f() public pure returns (uint, byte, byte) {
+		bytes memory encoded = abi.encodePacked("\\\\");
+		return (encoded.length, encoded[0], encoded[1]);
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// f() -> 2, 0x5c00000000000000000000000000000000000000000000000000000000000000, 0x5c00000000000000000000000000000000000000000000000000000000000000