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 [email protected] (Jingcheng Zhang)

Fixes golang#11824

Change-Id: I883a1982ea46465e1fb02e0e02b6a4df9e529ae4
Reviewed-on: https://go-review.googlesource.com/15015
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

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)

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