x/net: Possible error

[MAGeorg]

Go version

go version go1.21.5 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/georg/.cache/go-build'
GOENV='/home/georg/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/georg/go/pkg/mod'
GONOPROXY='https://git.aip.ooo/'
GONOSUMDB='https://git.aip.ooo/'
GOOS='linux'
GOPATH='/home/georg/go'
GOPRIVATE='https://git.aip.ooo/'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/georg/go/src/git.acronis.com/ab/account-server/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1679056629=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Hi!
I checked my project with the SAST tool and found some problems in the x/net code.

What did you see happen?

1. The problem is in x/net/internal/timeseries/timeseries/timeseries.go. On lines 369 and 377, there is a call to the extract function with the parameter num, which is used as the denominator in the division operation on line 398.

func (ts *timeSeries) ComputeRange(start, finish time.Time, num int) []Observable {
	if start.After(finish) {
		log.Printf("timeseries: start > finish, %v>%v", start, finish)
		return nil
	}

	if num < 0 {
		log.Printf("timeseries: num < 0, %v", num)
		return nil
	}

	results := make([]Observable, num)

	for _, l := range ts.levels {
		if !start.Before(l.end.Add(-l.size * time.Duration(ts.numBuckets))) {
			ts.extract(l, start, finish, num, results)
			return results
		}
	}

	// Failed to find a level that covers the desired range. So just
	// extract from the last level, even if it doesn't cover the entire
	// desired range.
	ts.extract(ts.levels[len(ts.levels)-1], start, finish, num, results)

	return results
}

func (ts *timeSeries) extract(l *tsLevel, start, finish time.Time, num int, results []Observable) {
	ts.mergePendingUpdates()

	srcInterval := l.size
	dstInterval := finish.Sub(start) / time.Duration(num)
...
}

The ComputeRange function is called on lines 334, where 1 is passed as num

func (ts *timeSeries) Range(start, finish time.Time) Observable {
	return ts.ComputeRange(start, finish, 1)[0]
}

and on line 389 in the ComputeRange function, where there is no check for num on 0.

func (ts *timeSeries) RecentList(delta time.Duration, num int) []Observable {
	if delta < 0 {
		return nil
	}
	now := ts.clock.Time()
	return ts.ComputeRange(now.Add(-delta), now, num)
}

In the extract function, there is also no check of num to 0.

2. The net/http2/server.go file. On line 2579, the function noteBodyReadFromHandler is called from b.conn, which is checked for nil above.

func (b *requestBody) Read(p []byte) (n int, err error) {
	if b.needsContinue {
		b.needsContinue = false
		b.conn.write100ContinueHeaders(b.stream)
	}
	if b.pipe == nil || b.sawEOF {
		return 0, io.EOF
	}
	n, err = b.pipe.Read(p)
	if err == io.EOF {
		b.sawEOF = true
	}
	if b.conn == nil && inTests {
		return
	}
	b.conn.noteBodyReadFromHandler(b.stream, n, err)
	return
}

Can it happen that b.conn is nil and the flag inTests is false? If such a situation is possible, it will get a dereferencing of the nil pointer.

p.s. The code references are given for version 0.39.0, but there are similar problems in version 0.28.0, which I use in the project (line numbers are different)

What did you expect to see?

Are the SAST results correct or is it a false positive?
Thanks!

[seankhliao]

I think until we see a real panic it probably isn't worth thinking too much about.