Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/text/unicode: possible array out of bounds from static analyzer result #66563

Open
nd-cyberprotect opened this issue Mar 27, 2024 · 6 comments
Open

x/text/unicode: possible array out of bounds from static analyzer result #66563

nd-cyberprotect opened this issue Mar 27, 2024 · 6 comments
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@nd-cyberprotect
Copy link

Go version

go version go1.21.6 linux/amd64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/me/.cache/go-build'
GOENV='/home/me/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/me/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/me/go'
GOPRIVATE=''
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.6'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/me/go/src/myproject/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-build3397856602=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Ran static analyzer

What did you see happen?

Got critical:

Accessing an element of array 'myproject/vendor/golang.org/x/text/unicode/norm.decomps' of size 19128 at forminfo.go:259 can lead to a buffer overflow, since the index 'v' can have an out of range value 32767, as indicated by a preceding conditional expression at forminfo.go:246.

What did you expect to see?

Please add control of index v values in range [len(decomps) : 32767] if it is necessary.
@gopherbot gopherbot added this to the Unreleased milestone Mar 27, 2024
@thanm thanm added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 27, 2024
@thanm
Copy link
Contributor

thanm commented Mar 27, 2024

Thanks for the report. It would be helpful to post which static analyzer tool you used.

Given that you found this problem, would you like to send a patch?

@thanm
Copy link
Contributor

thanm commented Mar 27, 2024

@mpvl

@nd-cyberprotect
Copy link
Author

It would be helpful to post which static analyzer tool you used.

Svace

@seankhliao seankhliao changed the title x/text: critical issue in v.0.14.0 from static analyzer x/text/unicode: possible array out of bounds from static analyzer result Mar 27, 2024
@nd-cyberprotect
Copy link
Author

Given that you found this problem, would you like to send a patch?

Is it enough to change 0x8000 to len(decomps) in x/text/unicode/norm/forminfo.go on line 246 like:

} else if v >= len(decomps) {

?

@adonovan
Copy link
Member

adonovan commented Mar 31, 2024 •
edited

as indicated by a preceding conditional expression

Yes, that looks like the right fix. Thanks.

It would be helpful to post which static analyzer tool you used.
Svace

I hadn't heard of Svace before. Apparently it is a closed-source, multi-language, deep (interprocedural, context-sensitive, path-sensitive, SMT) static analysis tool developed at the Russian Academy of Sciences, and Samsung has been using it for many years. See:

@thanm
Copy link
Contributor

thanm commented Mar 31, 2024

Samsung has been using it

TIL that Samsung has "90 different C/C++ compilers" being used in house. Yikes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@adonovan @gopherbot @thanm @nd-cyberprotect