- Backport of fix for CVE-2024-24791 to Go1.19

- https://go-review.googlesource.com/c/go/+/595096

Author: archanaravindar

patches/011-fix-CVE-2024-24791.patch

@@ -0,0 +1,331 @@
+From a5e9aab67d3c4ff3ded35495ae03fa34f12e950b Mon Sep 17 00:00:00 2001
+From: Archana Ravindar <[email protected]>
+Date: Mon, 12 Aug 2024 19:40:21 +0530
+Subject: [PATCH] - backport fix for CVE-2024-24791 to Go1.19 -
+ https://go-review.googlesource.com/c/go/+/595096
+
+---
+ src/net/http/server.go         |  25 ++--
+ src/net/http/transport.go      |  35 ++++--
+ src/net/http/transport_test.go | 205 ++++++++++++++++++++-------------
+ 3 files changed, 166 insertions(+), 99 deletions(-)
+
+diff --git a/src/net/http/server.go b/src/net/http/server.go
+index 87dd412984..fdcc3a9136 100644
+--- a/src/net/http/server.go
++++ b/src/net/http/server.go
+@@ -1344,16 +1344,21 @@ func (cw *chunkWriter) writeHeader(p []byte) {
+ 
+ 	// If the client wanted a 100-continue but we never sent it to
+ 	// them (or, more strictly: we never finished reading their
+-	// request body), don't reuse this connection because it's now
+-	// in an unknown state: we might be sending this response at
+-	// the same time the client is now sending its request body
+-	// after a timeout.  (Some HTTP clients send Expect:
+-	// 100-continue but knowing that some servers don't support
+-	// it, the clients set a timer and send the body later anyway)
+-	// If we haven't seen EOF, we can't skip over the unread body
+-	// because we don't know if the next bytes on the wire will be
+-	// the body-following-the-timer or the subsequent request.
+-	// See Issue 11549.
++	// request body), don't reuse this connection.
++	//
++	// This behavior was first added on the theory that we don't know
++	// if the next bytes on the wire are going to be the remainder of
++	// the request body or the subsequent request (see issue 11549),
++	// but that's not correct: If we keep using the connection,
++	// the client is required to send the request body whether we
++	// asked for it or not.
++	//
++	// We probably do want to skip reusing the connection in most cases,
++	// however. If the client is offering a large request body that we
++	// don't intend to use, then it's better to close the connection
++	// than to read the body. For now, assume that if we're sending
++	// headers, the handler is done reading the body and we should
++	// drop the connection if we haven't seen EOF.
+ 	if ecr, ok := w.req.Body.(*expectContinueReader); ok && !ecr.sawEOF.isSet() {
+ 		w.closeAfterReply = true
+ 	}
+diff --git a/src/net/http/transport.go b/src/net/http/transport.go
+index e470a6c080..083c907184 100644
+--- a/src/net/http/transport.go
++++ b/src/net/http/transport.go
+@@ -2288,17 +2288,12 @@ func (pc *persistConn) readResponse(rc requestAndChan, trace *httptrace.ClientTr
+ 			return
+ 		}
+ 		resCode := resp.StatusCode
+-		if continueCh != nil {
+-			if resCode == 100 {
+-				if trace != nil && trace.Got100Continue != nil {
+-					trace.Got100Continue()
+-				}
+-				continueCh <- struct{}{}
+-				continueCh = nil
+-			} else if resCode >= 200 {
+-				close(continueCh)
+-				continueCh = nil
++		if continueCh != nil && resCode == StatusContinue {
++			if trace != nil && trace.Got100Continue != nil {
++				trace.Got100Continue()
+ 			}
++			continueCh <- struct{}{}
++			continueCh = nil
+ 		}
+ 		is1xx := 100 <= resCode && resCode <= 199
+ 		// treat 101 as a terminal status, see issue 26161
+@@ -2322,6 +2317,26 @@ func (pc *persistConn) readResponse(rc requestAndChan, trace *httptrace.ClientTr
+ 		resp.Body = newReadWriteCloserBody(pc.br, pc.conn)
+ 	}
+ 
++	if continueCh != nil {
++		// We send an "Expect: 100-continue" header, but the server
++		// responded with a terminal status and no 100 Continue.
++		//
++		// If we're going to keep using the connection, we need to send the request body.
++		// Tell writeLoop to skip sending the body if we're going to close the connection,
++		// or to send it otherwise.
++		//
++		// The case where we receive a 101 Switching Protocols response is a bit
++		// ambiguous, since we don't know what protocol we're switching to.
++		// Conceivably, it's one that doesn't need us to send the body.
++		// Given that we'll send the body if ExpectContinueTimeout expires,
++		// be consistent and always send it if we aren't closing the connection.
++		if resp.Close || rc.req.Close {
++			close(continueCh) // don't send the body; the connection will close
++		} else {
++			continueCh <- struct{}{} // send the body
++		}
++	}
++
+ 	resp.TLS = pc.tlsState
+ 	return
+ }
+diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
+index 985d0625dc..63bcf0384b 100644
+--- a/src/net/http/transport_test.go
++++ b/src/net/http/transport_test.go
+@@ -1150,95 +1150,142 @@ func TestTransportGzip(t *testing.T) {
+ 	}
+ }
+ 
+-// If a request has Expect:100-continue header, the request blocks sending body until the first response.
+-// Premature consumption of the request body should not be occurred.
+-func TestTransportExpect100Continue(t *testing.T) {
+-	setParallel(t)
+-	defer afterTest(t)
++// A transport100Continue test exercises Transport behaviors when sending a
++// request with an Expect: 100-continue header.
++type transport100ContinueTest struct {
++	t *testing.T
+ 
+-	ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) {
+-		switch req.URL.Path {
+-		case "/100":
+-			// This endpoint implicitly responds 100 Continue and reads body.
+-			if _, err := io.Copy(io.Discard, req.Body); err != nil {
+-				t.Error("Failed to read Body", err)
+-			}
+-			rw.WriteHeader(StatusOK)
+-		case "/200":
+-			// Go 1.5 adds Connection: close header if the client expect
+-			// continue but not entire request body is consumed.
+-			rw.WriteHeader(StatusOK)
+-		case "/500":
+-			rw.WriteHeader(StatusInternalServerError)
+-		case "/keepalive":
+-			// This hijacked endpoint responds error without Connection:close.
+-			_, bufrw, err := rw.(Hijacker).Hijack()
+-			if err != nil {
+-				log.Fatal(err)
+-			}
+-			bufrw.WriteString("HTTP/1.1 500 Internal Server Error\r\n")
+-			bufrw.WriteString("Content-Length: 0\r\n\r\n")
+-			bufrw.Flush()
+-		case "/timeout":
+-			// This endpoint tries to read body without 100 (Continue) response.
+-			// After ExpectContinueTimeout, the reading will be started.
+-			conn, bufrw, err := rw.(Hijacker).Hijack()
+-			if err != nil {
+-				log.Fatal(err)
+-			}
+-			if _, err := io.CopyN(io.Discard, bufrw, req.ContentLength); err != nil {
+-				t.Error("Failed to read Body", err)
+-			}
+-			bufrw.WriteString("HTTP/1.1 200 OK\r\n\r\n")
+-			bufrw.Flush()
+-			conn.Close()
+-		}
++	reqdone chan struct{}
++	resp    *Response
++	respErr error
+ 
+-	}))
+-	defer ts.Close()
++	conn   net.Conn
++	reader *bufio.Reader
++}
+ 
+-	tests := []struct {
+-		path   string
+-		body   []byte
+-		sent   int
+-		status int
+-	}{
+-		{path: "/100", body: []byte("hello"), sent: 5, status: 200},       // Got 100 followed by 200, entire body is sent.
+-		{path: "/200", body: []byte("hello"), sent: 0, status: 200},       // Got 200 without 100. body isn't sent.
+-		{path: "/500", body: []byte("hello"), sent: 0, status: 500},       // Got 500 without 100. body isn't sent.
+-		{path: "/keepalive", body: []byte("hello"), sent: 0, status: 500}, // Although without Connection:close, body isn't sent.
+-		{path: "/timeout", body: []byte("hello"), sent: 5, status: 200},   // Timeout exceeded and entire body is sent.
++const transport100ContinueTestBody = "request body"
++
++// newTransport100ContinueTest creates a Transport and sends an Expect: 100-continue
++// request on it.
++func newTransport100ContinueTest(t *testing.T, timeout time.Duration) *transport100ContinueTest {
++	ln := newLocalListener(t)
++	defer ln.Close()
++
++	test := &transport100ContinueTest{
++		t:       t,
++		reqdone: make(chan struct{}),
+ 	}
+ 
+-	c := ts.Client()
+-	for i, v := range tests {
+-		tr := &Transport{
+-			ExpectContinueTimeout: 2 * time.Second,
+-		}
+-		defer tr.CloseIdleConnections()
+-		c.Transport = tr
+-		body := bytes.NewReader(v.body)
+-		req, err := NewRequest("PUT", ts.URL+v.path, body)
+-		if err != nil {
+-			t.Fatal(err)
+-		}
+-		req.Header.Set("Expect", "100-continue")
+-		req.ContentLength = int64(len(v.body))
++	tr := &Transport{
++		ExpectContinueTimeout: timeout,
++	}
++	go func() {
++		defer close(test.reqdone)
++		body := strings.NewReader(transport100ContinueTestBody)
++		req, _ := NewRequest("PUT", "http://"+ln.Addr().String(), body)
++				req.Header.Set("Expect", "100-continue")
++		req.ContentLength = int64(len(transport100ContinueTestBody))
++		test.resp, test.respErr = tr.RoundTrip(req)
++		test.resp.Body.Close()
++	}()
+ 
+-		resp, err := c.Do(req)
+-		if err != nil {
+-			t.Fatal(err)
++	c, err := ln.Accept()
++	if err != nil {
++		t.Fatalf("Accept: %v", err)
++	}
++	t.Cleanup(func() {
++		c.Close()
++	})
++	br := bufio.NewReader(c)
++	_, err = ReadRequest(br)
++	if err != nil {
++		t.Fatalf("ReadRequest: %v", err)
++	}
++	test.conn = c
++	test.reader = br
++	t.Cleanup(func() {
++		<-test.reqdone
++		tr.CloseIdleConnections()
++		got, _ := io.ReadAll(test.reader)
++		if len(got) > 0 {
++			t.Fatalf("Transport sent unexpected bytes: %q", got)
+ 		}
+-		resp.Body.Close()
++	})
+ 
+-		sent := len(v.body) - body.Len()
+-		if v.status != resp.StatusCode {
+-			t.Errorf("test %d: status code should be %d but got %d. (%s)", i, v.status, resp.StatusCode, v.path)
+-		}
+-		if v.sent != sent {
+-			t.Errorf("test %d: sent body should be %d but sent %d. (%s)", i, v.sent, sent, v.path)
++	return test
++}
++
++// respond sends response lines from the server to the transport.
++func (test *transport100ContinueTest) respond(lines ...string) {
++	for _, line := range lines {
++		if _, err := test.conn.Write([]byte(line + "\r\n")); err != nil {
++			test.t.Fatalf("Write: %v", err)
+ 		}
+ 	}
++	if _, err := test.conn.Write([]byte("\r\n")); err != nil {
++		test.t.Fatalf("Write: %v", err)
++	}
++}
++
++// wantBodySent ensures the transport has sent the request body to the server.
++func (test *transport100ContinueTest) wantBodySent() {
++	got, err := io.ReadAll(io.LimitReader(test.reader, int64(len(transport100ContinueTestBody))))
++	if err != nil {
++		test.t.Fatalf("unexpected error reading body: %v", err)
++	}
++	if got, want := string(got), transport100ContinueTestBody; got != want {
++		test.t.Fatalf("unexpected body: got %q, want %q", got, want)
++	}
++}
++
++// wantRequestDone ensures the Transport.RoundTrip has completed with the expected status.
++func (test *transport100ContinueTest) wantRequestDone(want int) {
++	<-test.reqdone
++	if test.respErr != nil {
++		test.t.Fatalf("unexpected RoundTrip error: %v", test.respErr)
++	}
++	if got := test.resp.StatusCode; got != want {
++		test.t.Fatalf("unexpected response code: got %v, want %v", got, want)
++	}
++}
++
++func TestTransportExpect100ContinueSent(t *testing.T) {
++	test := newTransport100ContinueTest(t, 1*time.Hour)
++	// Server sends a 100 Continue response, and the client sends the request body.
++	test.respond("HTTP/1.1 100 Continue")
++	test.wantBodySent()
++	test.respond("HTTP/1.1 200", "Content-Length: 0")
++	test.wantRequestDone(200)
++}
++
++func TestTransportExpect100Continue200ResponseNoConnClose(t *testing.T) {
++	test := newTransport100ContinueTest(t, 1*time.Hour)
++	// No 100 Continue response, no Connection: close header.
++	test.respond("HTTP/1.1 200", "Content-Length: 0")
++	test.wantBodySent()
++	test.wantRequestDone(200)
++}
++
++func TestTransportExpect100Continue200ResponseWithConnClose(t *testing.T) {
++	test := newTransport100ContinueTest(t, 1*time.Hour)
++	// No 100 Continue response, Connection: close header set.
++	test.respond("HTTP/1.1 200", "Connection: close", "Content-Length: 0")
++	test.wantRequestDone(200)
++}
++
++func TestTransportExpect100Continue500ResponseNoConnClose(t *testing.T) {
++	test := newTransport100ContinueTest(t, 1*time.Hour)
++	// No 100 Continue response, no Connection: close header.
++	test.respond("HTTP/1.1 500", "Content-Length: 0")
++	test.wantBodySent()
++	test.wantRequestDone(500)
++}
++
++func TestTransportExpect100Continue500ResponseTimeout(t *testing.T) {
++	test := newTransport100ContinueTest(t, 5*time.Millisecond) // short timeout
++	test.wantBodySent()                                        // after timeout
++	test.respond("HTTP/1.1 200", "Content-Length: 0")
++	test.wantRequestDone(200)
+ }
+ 
+ func TestSOCKS5Proxy(t *testing.T) {
+-- 
+2.39.3
+