aws/signer/v4: Fix out of bounds panic in stripExcessSpaces (#1412)

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

@@ -614,8 +614,8 @@ func (ctx *signingCtx) buildCanonicalHeaders(r rule, header http.Header) {
 				strings.Join(ctx.SignedHeaderVals[k], ",")
 		}
 	}
-
-	ctx.canonicalHeaders = strings.Join(stripExcessSpaces(headerValues), "\n")
+	stripExcessSpaces(headerValues)
+	ctx.canonicalHeaders = strings.Join(headerValues, "\n")
 }
 
 func (ctx *signingCtx) buildCanonicalString() {
@@ -721,9 +721,10 @@ const doubleSpaces = "  "
 
 var doubleSpaceBytes = []byte(doubleSpaces)
 
-func stripExcessSpaces(headerVals []string) []string {
-	vals := make([]string, len(headerVals))
-	for i, str := range headerVals {
+// stripExcessSpaces will rewrite the passed in slice's string values to not
+// contain muliple side-by-side spaces.
+func stripExcessSpaces(vals []string) {
+	for i, str := range vals {
 		// Trim leading and trailing spaces
 		trimmed := strings.TrimSpace(str)
 
@@ -735,27 +736,28 @@ func stripExcessSpaces(headerVals []string) []string {
 
 		buf := []byte(trimmed)
 		for idx > -1 {
-			stripToIdx := -1
-			for j := idx + 1; j < len(buf); j++ {
+			idx++ // Start on the second space
+
+			stripped := false
+			for j := idx; j < len(buf); j++ {
 				if buf[j] != ' ' {
-					buf = append(buf[:idx+1], buf[j:]...)
-					stripToIdx = j - idx - 1
+					buf = append(buf[:idx], buf[j:]...)
+					stripped = true
 					break
 				}
 			}
+			if !stripped {
+				break
+			}
 
-			if stripToIdx >= 0 {
-				// Find next double space
-				idx = bytes.Index(buf[stripToIdx:], doubleSpaceBytes)
-				if idx >= 0 {
-					idx += stripToIdx
-				}
-			} else {
-				idx = -1
+			// Find next double space
+			origIdx := idx
+			idx = bytes.Index(buf[idx:], doubleSpaceBytes)
+			if idx > 0 {
+				idx += origIdx
 			}
 		}
 
 		vals[i] = string(buf)
 	}
-	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,29 @@ 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)
+		// Make sure to start with a copy of the cases
+		cases := append([]string{}, stripExcessSpaceCases...)
+		stripExcessSpaces(cases)
 	}
 }