Prevent chunk extension request smuggling (#2165)

header.go

@@ -2677,15 +2677,6 @@ func (h *RequestHeader) parse(buf []byte) (int, error) {
 }
 
 func parseTrailer(src []byte, dest []argsKV, disableNormalizing bool) ([]argsKV, int, error) {
-	// Skip any 0 length chunk.
-	if src[0] == '0' {
-		skip := len(strCRLF) + 1
-		if len(src) < skip {
-			return dest, 0, io.EOF
-		}
-		src = src[skip:]
-	}
-
 	var s headerScanner
 	s.b = src
 

http.go

@@ -1454,7 +1454,10 @@ func (req *Request) ContinueReadBody(r *bufio.Reader, maxBodySize int, preParseM
 
 	if contentLength == -1 {
 		err = req.Header.ReadTrailer(r)
-		if err != nil && err != io.EOF {
+		if err != nil {
+			if err == io.EOF {
+				return ErrBrokenChunk{error: io.ErrUnexpectedEOF}
+			}
 			return err
 		}
 	}
@@ -1592,7 +1595,10 @@ func (resp *Response) ReadLimitBody(r *bufio.Reader, maxBodySize int) error {
 	// A response without a body can't have trailers.
 	if resp.Header.ContentLength() == -1 && !resp.StreamBody && !resp.mustSkipBody() {
 		err = resp.Header.ReadTrailer(r)
-		if err != nil && err != io.EOF {
+		if err != nil {
+			if err == io.EOF {
+				return ErrBrokenChunk{error: io.ErrUnexpectedEOF}
+			}
 			return err
 		}
 	}
@@ -2743,6 +2749,7 @@ func parseChunkSize(r *bufio.Reader) (int, error) {
 	if err != nil {
 		return -1, err
 	}
+	inExt := false
 	for {
 		c, err := r.ReadByte()
 		if err != nil {
@@ -2750,24 +2757,35 @@ func parseChunkSize(r *bufio.Reader) (int, error) {
 				error: fmt.Errorf("cannot read '\\r' char at the end of chunk size: %w", err),
 			}
 		}
-		// Skip chunk extension after chunk size.
-		// Add support later if anyone needs it.
-		if c != '\r' {
-			// Security: Don't allow newlines in chunk extensions.
-			// This can lead to request smuggling issues with some reverse proxies.
-			if c == '\n' {
+		if c == '\r' {
+			if err := r.UnreadByte(); err != nil {
 				return -1, ErrBrokenChunk{
-					error: errors.New("invalid character '\\n' after chunk size"),
+					error: fmt.Errorf("cannot unread '\\r' char at the end of chunk size: %w", err),
 				}
 			}
-			continue
+			break
 		}
-		if err := r.UnreadByte(); err != nil {
+		// Security: Don't allow newlines in chunk extensions.
+		// This can lead to request smuggling issues with some reverse proxies.
+		if c == '\n' {
 			return -1, ErrBrokenChunk{
-				error: fmt.Errorf("cannot unread '\\r' char at the end of chunk size: %w", err),
+				error: errors.New("invalid character '\\n' after chunk size"),
+			}
+		}
+		if inExt {
+			continue
+		}
+		switch c {
+		case ' ', '\t':
+			continue
+		case ';':
+			inExt = true
+			continue
+		default:
+			return -1, ErrBrokenChunk{
+				error: fmt.Errorf("invalid character %q after chunk size", c),
 			}
 		}
-		break
 	}
 	err = readCrLf(r)
 	if err != nil {

http_test.go

@@ -3245,6 +3245,12 @@ func TestRequestMultipartFormPipeEmptyFormField(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
+	if _, err = bw.Write(strCRLF); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if err = bw.Flush(); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
 
 	for e := range errs {
 		t.Fatalf("unexpected error in goroutine multiwriter: %v", e)