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

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)
 	}
 }