signer, log: properly escape character sequences (#20987)

* signer: properly handle terminal escape characters

* log: use strconv conversion instead of custom escape function

* log: remove relection tests for nil

log/format.go

@@ -358,49 +358,19 @@ func formatLogfmtValue(value interface{}, term bool) string {
 	}
 }
 
-var stringBufPool = sync.Pool{
+// escapeString checks if the provided string needs escaping/quoting, and
-	New: func() interface{} { return new(bytes.Buffer) },
+// calls strconv.Quote if needed
-}
-
 func escapeString(s string) string {
-	needsQuotes := false
+	needsQuoting := false
-	needsEscape := false
 	for _, r := range s {
-		if r <= ' ' || r == '=' || r == '"' {
+		// We quote everything below " (0x34) and above~ (0x7E), plus equal-sign
-			needsQuotes = true
+		if r <= '"' || r > '~' || r == '=' {
-		}
+			needsQuoting = true
-		if r == '\\' || r == '"' || r == '\n' || r == '\r' || r == '\t' {
+			break
-			needsEscape = true
 		}
 	}
-	if !needsEscape && !needsQuotes {
+	if !needsQuoting {
 		return s
 	}
-	e := stringBufPool.Get().(*bytes.Buffer)
+	return strconv.Quote(s)
-	e.WriteByte('"')
-	for _, r := range s {
-		switch r {
-		case '\\', '"':
-			e.WriteByte('\\')
-			e.WriteByte(byte(r))
-		case '\n':
-			e.WriteString("\\n")
-		case '\r':
-			e.WriteString("\\r")
-		case '\t':
-			e.WriteString("\\t")
-		default:
-			e.WriteRune(r)
-		}
-	}
-	e.WriteByte('"')
-	var ret string
-	if needsQuotes {
-		ret = e.String()
-	} else {
-		ret = string(e.Bytes()[1 : e.Len()-1])
-	}
-	e.Reset()
-	stringBufPool.Put(e)
-	return ret
 }

signer/core/cliui.go

@@ -85,10 +85,19 @@ func (ui *CommandlineUI) confirm() bool {
 	return false
 }
 
+// sanitize quotes and truncates 'txt' if longer than 'limit'. If truncated,
+// and ellipsis is added after the quoted string
+func sanitize(txt string, limit int) string {
+	if len(txt) > limit {
+		return fmt.Sprintf("%q...", txt[:limit])
+	}
+	return fmt.Sprintf("%q", txt)
+}
+
 func showMetadata(metadata Metadata) {
 	fmt.Printf("Request context:\n\t%v -> %v -> %v\n", metadata.Remote, metadata.Scheme, metadata.Local)
 	fmt.Printf("\nAdditional HTTP header data, provided by the external caller:\n")
-	fmt.Printf("\tUser-Agent: %v\n\tOrigin: %v\n", metadata.UserAgent, metadata.Origin)
+	fmt.Printf("\tUser-Agent: %v\n\tOrigin: %v\n", sanitize(metadata.UserAgent, 200), sanitize(metadata.Origin, 100))
 }
 
 // ApproveTx prompt the user for confirmation to request to sign Transaction
@@ -113,7 +122,6 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest) (SignTxResponse, erro
 	if request.Transaction.Data != nil {
 		d := *request.Transaction.Data
 		if len(d) > 0 {
-
 			fmt.Printf("data:     %v\n", hexutil.Encode(d))
 		}
 	}
@@ -145,7 +153,7 @@ func (ui *CommandlineUI) ApproveSignData(request *SignDataRequest) (SignDataResp
 	for _, nvt := range request.Messages {
 		fmt.Printf("\u00a0\u00a0%v\n", strings.TrimSpace(nvt.Pprint(1)))
 	}
-	fmt.Printf("raw data:  \n%q\n", request.Rawdata)
+	fmt.Printf("raw data:  \n\t%q\n", request.Rawdata)
 	fmt.Printf("data hash:  %v\n", request.Hash)
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(request.Meta)

signer/core/signed_data.go

@@ -827,23 +827,23 @@ func (t Types) validate() error {
 		}
 		for i, typeObj := range typeArr {
 			if len(typeObj.Type) == 0 {
-				return fmt.Errorf("type %v:%d: empty Type", typeKey, i)
+				return fmt.Errorf("type %q:%d: empty Type", typeKey, i)
 			}
 			if len(typeObj.Name) == 0 {
-				return fmt.Errorf("type %v:%d: empty Name", typeKey, i)
+				return fmt.Errorf("type %q:%d: empty Name", typeKey, i)
 			}
 			if typeKey == typeObj.Type {
-				return fmt.Errorf("type '%s' cannot reference itself", typeObj.Type)
+				return fmt.Errorf("type %q cannot reference itself", typeObj.Type)
 			}
 			if typeObj.isReferenceType() {
 				if _, exist := t[typeObj.typeName()]; !exist {
-					return fmt.Errorf("reference type '%s' is undefined", typeObj.Type)
+					return fmt.Errorf("reference type %q is undefined", typeObj.Type)
 				}
 				if !typedDataReferenceTypeRegexp.MatchString(typeObj.Type) {
-					return fmt.Errorf("unknown reference type '%s", typeObj.Type)
+					return fmt.Errorf("unknown reference type %q", typeObj.Type)
 				}
 			} else if !isPrimitiveTypeValid(typeObj.Type) {
-				return fmt.Errorf("unknown type '%s'", typeObj.Type)
+				return fmt.Errorf("unknown type %q", typeObj.Type)
 			}
 		}
 	}

signer/fourbyte/abi.go

@@ -85,7 +85,7 @@ var selectorRegexp = regexp.MustCompile(`^([^\)]+)\(([A-Za-z0-9,\[\]]*)\)`)
 
 // parseSelector converts a method selector into an ABI JSON spec. The returned
 // data is a valid JSON string which can be consumed by the standard abi package.
-func parseSelector(selector string) ([]byte, error) {
+func parseSelector(unescapedSelector string) ([]byte, error) {
 	// Define a tiny fake ABI struct for JSON marshalling
 	type fakeArg struct {
 		Type string `json:"type"`
@@ -95,10 +95,10 @@ func parseSelector(selector string) ([]byte, error) {
 		Type   string    `json:"type"`
 		Inputs []fakeArg `json:"inputs"`
 	}
-	// Validate the selector and extract it's components
+	// Validate the unescapedSelector and extract it's components
-	groups := selectorRegexp.FindStringSubmatch(selector)
+	groups := selectorRegexp.FindStringSubmatch(unescapedSelector)
 	if len(groups) != 3 {
-		return nil, fmt.Errorf("invalid selector %s (%v matches)", selector, len(groups))
+		return nil, fmt.Errorf("invalid selector %q (%v matches)", unescapedSelector, len(groups))
 	}
 	name := groups[1]
 	args := groups[2]
@@ -115,7 +115,7 @@ func parseSelector(selector string) ([]byte, error) {
 
 // parseCallData matches the provided call data against the ABI definition and
 // returns a struct containing the actual go-typed values.
-func parseCallData(calldata []byte, abidata string) (*decodedCallData, error) {
+func parseCallData(calldata []byte, unescapedAbidata string) (*decodedCallData, error) {
 	// Validate the call data that it has the 4byte prefix and the rest divisible by 32 bytes
 	if len(calldata) < 4 {
 		return nil, fmt.Errorf("invalid call data, incomplete method signature (%d bytes < 4)", len(calldata))
@@ -127,9 +127,9 @@ func parseCallData(calldata []byte, abidata string) (*decodedCallData, error) {
 		return nil, fmt.Errorf("invalid call data; length should be a multiple of 32 bytes (was %d)", len(argdata))
 	}
 	// Validate the called method and upack the call data accordingly
-	abispec, err := abi.JSON(strings.NewReader(abidata))
+	abispec, err := abi.JSON(strings.NewReader(unescapedAbidata))
 	if err != nil {
-		return nil, fmt.Errorf("invalid method signature (%s): %v", abidata, err)
+		return nil, fmt.Errorf("invalid method signature (%q): %v", unescapedAbidata, err)
 	}
 	method, err := abispec.MethodById(sigdata)
 	if err != nil {

signer/fourbyte/validation.go

@@ -98,7 +98,7 @@ func (db *Database) ValidateCallData(selector *string, data []byte, messages *co
 		if info, err := verifySelector(*selector, data); err != nil {
 			messages.Warn(fmt.Sprintf("Transaction contains data, but provided ABI signature could not be matched: %v", err))
 		} else {
-			messages.Info(info.String())
+			messages.Info(fmt.Sprintf("Transaction invokes the following method: %q", info.String()))
 			db.AddSelector(*selector, data[:4])
 		}
 		return
@@ -112,6 +112,6 @@ func (db *Database) ValidateCallData(selector *string, data []byte, messages *co
 	if info, err := verifySelector(embedded, data); err != nil {
 		messages.Warn(fmt.Sprintf("Transaction contains data, but provided ABI signature could not be verified: %v", err))
 	} else {
-		messages.Info(info.String())
+		messages.Info(fmt.Sprintf("Transaction invokes the following method: %q", info.String()))
 	}
 }