net/http/fcgi: fix panic with malformed params record

As stated in FastCGI specifications:

FastCGI transmits a name-value pair as the length of the name,
followed by the length of the value, followed by the name,
followed by the value.

The current implementation trusts the name and value length
provided in the record, leading to a panic if the record
is malformed.

Added an explicit check on the lengths.

Test case and fix suggested by diogin@gmail.com (Jingcheng Zhang)

Fixes #11824

Change-Id: I883a1982ea46465e1fb02e0e02b6a4df9e529ae4
Reviewed-on: https://go-review.googlesource.com/15015
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/fcgi/child.go b/src/net/http/fcgi/child.go
index da824ed717e14e257fead4fc58b6550a776d522e..88704245db8742325c426fcad49ecd37e1c8606e 100644 (file)
--- a/src/net/http/fcgi/child.go
+++ b/src/net/http/fcgi/child.go
@@ -56,6 +56,9 @@ func (r *request) parseParams() {
                        return
                }
                text = text[n:]
+               if int(keyLen)+int(valLen) > len(text) {
+                       return
+               }
                key := readString(text, keyLen)
                text = text[keyLen:]
                val := readString(text, valLen)

diff --git a/src/net/http/fcgi/fcgi_test.go b/src/net/http/fcgi/fcgi_test.go
index de0f7f831f61683a34dd6be8619f25e6e341002a..b6013bfdd519362f71e91f9c79d67991dc4a3f9e 100644 (file)
--- a/src/net/http/fcgi/fcgi_test.go
+++ b/src/net/http/fcgi/fcgi_test.go
@@ -254,3 +254,27 @@ func TestChildServeCleansUp(t *testing.T) {
                <-done
        }
 }
+
+type rwNopCloser struct {
+       io.Reader
+       io.Writer
+}
+
+func (rwNopCloser) Close() error {
+       return nil
+}
+
+// Verifies it doesn't crash.  Issue 11824.
+func TestMalformedParams(t *testing.T) {
+       input := []byte{
+               // beginRequest, requestId=1, contentLength=8, role=1, keepConn=1
+               1, 1, 0, 1, 0, 8, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0,
+               // params, requestId=1, contentLength=10, k1Len=50, v1Len=50 (malformed, wrong length)
+               1, 4, 0, 1, 0, 10, 0, 0, 50, 50, 3, 4, 5, 6, 7, 8, 9, 10,
+               // end of params
+               1, 4, 0, 1, 0, 0, 0, 0,
+       }
+       rw := rwNopCloser{bytes.NewReader(input), ioutil.Discard}
+       c := newChild(rw, http.DefaultServeMux)
+       c.serve()
+}