Skip to content
This repository has been archived by the owner on Sep 17, 2019. It is now read-only.

nil pointer dereference in hpack.HuffmanDecode #56

Closed
dvyukov opened this issue May 4, 2015 · 2 comments
Closed

nil pointer dereference in hpack.HuffmanDecode #56

dvyukov opened this issue May 4, 2015 · 2 comments

Comments

@dvyukov
Copy link

dvyukov commented May 4, 2015

The following test crashes with nil deref:

package http2

import (
    "io"
    "net"
    "net/http"
    "testing"
    "time"
)

var data = "PRI * HTTP/2.0\r\n\r\nSM" +
    "\r\n\r\n\x00\x00\x00\x040\x00\x00\x00\x00\x00\x00\x14\x01\x0000" +
    "0\xbf\x00\x8800\x91\xff\xff\xff\xff\xc800000000" +
    "00"

func TestFuzz(t *testing.T) {
    s := &http.Server{}
    s2 := &Server{MaxReadFrameSize: 1 << 16, PermitProhibitedCipherSuites: true}
    c := &MyConn{[]byte(data), false, false}
    s2.handleConn(s, c, http.HandlerFunc(handler))
    if !c.closed {
        panic("connection is not closed")
    }
}

func handler(w http.ResponseWriter, req *http.Request) {
    w.Write([]byte("hello"))
}

type MyConn struct {
    data    []byte
    closed  bool
    written bool
}

func (c *MyConn) Read(b []byte) (n int, err error) {
    if len(c.data) == 0 {
        return 0, io.EOF
    }
    n = copy(b, c.data)
    c.data = c.data[n:]
    return
}

func (c *MyConn) Write(b []byte) (n int, err error) {
    c.written = true
    return len(b), nil
}

func (c *MyConn) Close() error {
    c.closed = true
    return nil
}

func (c *MyConn) LocalAddr() net.Addr {
    return &net.TCPAddr{net.IP{127, 0, 0, 1}, 49706, ""}
}

func (c *MyConn) RemoteAddr() net.Addr {
    return &net.TCPAddr{net.IP{127, 0, 0, 1}, 49706, ""}
}

func (c *MyConn) SetDeadline(t time.Time) error {
    return nil
}

func (c *MyConn) SetReadDeadline(t time.Time) error {
    return nil
}

func (c *MyConn) SetWriteDeadline(t time.Time) error {
    return nil
}
panic: runtime error: invalid memory address or nil pointer dereference

goroutine 5 [running]:
testing.tRunner.func1(0xc208074120)
    testing/testing.go:446 +0x174
github.com/bradfitz/http2/hpack.HuffmanDecode(0x768880, 0xc208016540, 0xc2080ae822, 0x8, 0x12, 0x0, 0x0, 0x0)
    http2/hpack/huffman.go:33 +0x1cd
github.com/bradfitz/http2/hpack.readString(0xc2080ae822, 0x12, 0x12, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    http2/hpack/hpack.go:441 +0x35f
github.com/bradfitz/http2/hpack.(*Decoder).parseFieldLiteral(0xc2080dc000, 0x4, 0x1, 0x0, 0x0)
    http2/hpack/hpack.go:347 +0x576
github.com/bradfitz/http2/hpack.(*Decoder).parseHeaderFieldRepr(0xc2080dc000, 0x0, 0x0)
    http2/hpack/hpack.go:299 +0x11f
github.com/bradfitz/http2/hpack.(*Decoder).Write(0xc2080dc000, 0xc2080ae820, 0x14, 0x14, 0x0, 0x0, 0x0)
    http2/hpack/hpack.go:249 +0xbd
github.com/bradfitz/http2.(*serverConn).processHeaderBlockFragment(0xc208086280, 0xc208016460, 0xc2080ae820, 0x14, 0x14, 0x302900, 0x0, 0x0)
    http2/server.go:1257 +0x83
github.com/bradfitz/http2.(*serverConn).processHeaders(0xc208086280, 0xc20800b110, 0x0, 0x0)
    http2/server.go:1243 +0x47e
github.com/bradfitz/http2.(*serverConn).processFrame(0xc208086280, 0x7689e0, 0xc20800b110, 0x0, 0x0)
    http2/server.go:970 +0x554
github.com/bradfitz/http2.(*serverConn).processFrameFromReader(0xc208086280, 0x7689e0, 0xc20800b110, 0xc20801a900, 0x1, 0x768801)
    http2/server.go:919 +0x3b6
github.com/bradfitz/http2.(*serverConn).serve(0xc208086280)
    http2/server.go:643 +0x9cb
github.com/bradfitz/http2.(*Server).handleConn(0xc2080ae6c0, 0xc20801a4e0, 0x7687a0, 0xc2080ae6e0, 0x768800, 0x4facc8)
    http2/server.go:277 +0x9ef
github.com/bradfitz/http2.TestFuzz(0xc208074120)
    http2/fuzz_test.go:20 +0x1ad

on commit 9516364

@dvyukov
Copy link
Author

dvyukov commented May 4, 2015

Similar crasher. Crash happens in processContinuation instead of processHeaders.

    "PRI * HTTP/2.0\r\n\r\nSM" +
    "\r\n\r\n\x00\x00\x00\x040\x00\x00\x00\x00\x00\x00\xf1\x01\x00nf" +
    "i\r00\x00\x00\t0000000000000" +
    "\x00\x010\t00000000000\n\x00\x00\xdf0" +
    "00000000000000000000" +
    "00000000000000000000" +
    "00000000000000000000" +
    "00000000000000000000" +
    "00000000000000\xf300000" +
    "00000000000000000000" +
    "00000000000000000000" +
    "0000000000000000\r0\x10\x00" +
    "0\u007f\xff\xff\xff000000000000000" +
    "00000000000000000000" +
    "000\x00\x00\x19\t0nfi\r00000000" +
    "00000000000000000"

@bradfitz
Copy link
Owner

Fix out for review: https://go-review.googlesource.com/#/c/15738

c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
Return an error instead.

Fixes bradfitz/http2#56

Change-Id: I3d1e80a214a8635932479943f0ef9610ee02b233
Reviewed-on: https://go-review.googlesource.com/15738
Reviewed-by: Andrew Gerrand <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants