aws/signer/v4: Fix out of bounds panic in stripExcessSpaces

Fixes the out of bands panic in stripExcessSpaces caused by an incorrect
calculation of the stripToIdx value. Simplified to code also.

Updates stripExcessSpaces to trim the string slice inline instead of
creating a new slice.

Fix #1411

aws/signer/v4/v4.go

@@ -55,7 +55,6 @@
 package v4
 
 import (
-	"bytes"
 	"crypto/hmac"
 	"crypto/sha256"
 	"encoding/hex"
@@ -615,7 +614,8 @@ func (ctx *signingCtx) buildCanonicalHeaders(r rule, header http.Header) {
 		}
 	}
 
-	ctx.canonicalHeaders = strings.Join(stripExcessSpaces(headerValues), "\n")
+	stripExcessSpaces(headerValues)
+	ctx.canonicalHeaders = strings.Join(headerValues, "\n")
 }
 
 func (ctx *signingCtx) buildCanonicalString() {
@@ -717,13 +717,12 @@ func makeSha256Reader(reader io.ReadSeeker) []byte {
 	return hash.Sum(nil)
 }
 
-const doubleSpaces = "  "
+// stripExcessSpaces will rewrite the passed in slice's string values to not
+// contain muliple side-by-side spaces.
+func stripExcessSpaces(vals []string) {
+	const doubleSpaces = "  "
 
-var doubleSpaceBytes = []byte(doubleSpaces)
-
-func stripExcessSpaces(headerVals []string) []string {
-	vals := make([]string, len(headerVals))
-	for i, str := range headerVals {
+	for i, str := range vals {
 		// Trim leading and trailing spaces
 		trimmed := strings.TrimSpace(str)
 
@@ -733,29 +732,7 @@ func stripExcessSpaces(headerVals []string) []string {
 			continue
 		}
 
-		buf := []byte(trimmed)
-		for idx > -1 {
-			stripToIdx := -1
-			for j := idx + 1; j < len(buf); j++ {
-				if buf[j] != ' ' {
-					buf = append(buf[:idx+1], buf[j:]...)
-					stripToIdx = j - idx - 1
-					break
-				}
-			}
-
-			if stripToIdx >= 0 {
-				// Find next double space
-				idx = bytes.Index(buf[stripToIdx:], doubleSpaceBytes)
-				if idx >= 0 {
-					idx += stripToIdx
-				}
-			} else {
-				idx = -1
-			}
-		}
-
-		vals[i] = string(buf)
+		// Only tokenize the string if there were double spaces
+		vals[i] = strings.Join(strings.Fields(trimmed), " ")
 	}
-	return vals
 }

aws/signer/v4/v4_test.go

@@ -30,6 +30,8 @@ func TestStripExcessHeaders(t *testing.T) {
 		" 1  2  ",
 		"12   3",
 		"12   3   1",
+		"12           3     1",
+		"12     3       1abc123",
 	}
 
 	expected := []string{
@@ -43,11 +45,13 @@ func TestStripExcessHeaders(t *testing.T) {
 		"1 2",
 		"12 3",
 		"12 3 1",
+		"12 3 1",
+		"12 3 1abc123",
 	}
 
-	newVals := stripExcessSpaces(vals)
-	for i := 0; i < len(newVals); i++ {
-		assert.Equal(t, expected[i], newVals[i], "test: %d", i)
+	stripExcessSpaces(vals)
+	for i := 0; i < len(vals); i++ {
+		assert.Equal(t, expected[i], vals[i], "test: %d", i)
 	}
 }
 
@@ -507,15 +511,27 @@ func BenchmarkSignRequest(b *testing.B) {
 	}
 }
 
-func BenchmarkStripExcessSpaces(b *testing.B) {
-	vals := []string{
-		`AWS4-HMAC-SHA256 Credential=AKIDFAKEIDFAKEID/20160628/us-west-2/s3/aws4_request, SignedHeaders=host;x-amz-date, Signature=1234567890abcdef1234567890abcdef1234567890abcdef`,
-		`123   321   123   321`,
-		`   123   321   123   321   `,
-	}
+var stripExcessSpaceCases = []string{
+	`AWS4-HMAC-SHA256 Credential=AKIDFAKEIDFAKEID/20160628/us-west-2/s3/aws4_request, SignedHeaders=host;x-amz-date, Signature=1234567890abcdef1234567890abcdef1234567890abcdef`,
+	`123   321   123   321`,
+	`   123   321   123   321   `,
+	`   123    321    123          321   `,
+	"123",
+	"1 2 3",
+	"  1 2 3",
+	"1  2 3",
+	"1  23",
+	"1  2  3",
+	"1  2  ",
+	" 1  2  ",
+	"12   3",
+	"12   3   1",
+	"12           3     1",
+	"12     3       1abc123",
+}
 
-	b.ResetTimer()
+func BenchmarkStripExcessSpaces(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		stripExcessSpaces(vals)
+		stripExcessSpaces(stripExcessSpaceCases)
 	}
 }