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.

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

x/image/tiff: corrupt or malicious paletted images parse successfully and later panic in (*Palleted).At #67624

Closed
jswright opened this issue May 23, 2024 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@jswright
Copy link

Go version

go version go1.23-20240419-RC02 cl/626470163 +7f76c00fc5 X:fieldtrack,boringcrypto linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/usr/local/google/home/jsw/.cache/go-build'
GOENV='/usr/local/google/home/jsw/.config/go/env'
GOEXE=''
GOEXPERIMENT='fieldtrack,boringcrypto'
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/usr/local/google/home/jsw/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/usr/local/google/home/jsw/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/google-golang'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/google-golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23-20240419-RC02 cl/626470163 +7f76c00fc5 X:fieldtrack,boringcrypto'
GODEBUG=''
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='0'
GOMOD='/usr/local/google/home/jsw/src/go/image/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 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build937185890=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Reproducer:

package main

import (
	"fmt"
	"os"

	"golang.org/x/image/tiff"
)

func main() {
	if len(os.Args) != 2 {
		fmt.Fprintf(os.Stderr, "Usage: %v <filename>")
		os.Exit(1)
	}
	f, err := os.Open(os.Args[1])
	if err != nil {
		panic(err)
	}
	defer f.Close()

	img, err := tiff.Decode(f)
	if err != nil {
		panic(err)
	}

	b := img.Bounds()
	for x := b.Min.X; x <= b.Max.X; x++ {
		for y := b.Min.Y; y <= b.Max.Y; y++ {
			_ = img.At(x, y)
		}
	}
}

What did you see happen?

Running the above reproducer with a copy of https://github.com/pic4xiu/pocRep/blob/main/poc.tiff:

panic: runtime error: index out of range [70] with length 65

goroutine 1 [running]:
image.(*Paletted).At(0x4f16b8?, 0xc00005a038?, 0x0?)
	/usr/lib/google-golang/src/image/image.go:1173 +0x9a
main.main()
	/usr/local/google/home/jsw/src/go/image-tiff-repro/main.go:29 +0x15f

What did you expect to see?

I'd expect to see the parser return an error instead of leniently parsing and causing a panic when the user actually tries to use the resulting image.


Context: https://osv.dev/vulnerability/GHSA-q7pp-wcgr-pffx, based on disintegration/imaging#165. The actual issue is not in that library but in this TIFF parser.


I have a proposed fix that I will send a code review for shortly. (It just checks each index against the palette size before calling img.SetColorIndex.)

@gopherbot gopherbot added this to the Unreleased milestone May 23, 2024
@ianlancetaylor
Copy link
Contributor

CC @nigeltao

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/588115 mentions this issue: tiff: Validate palette indices when parsing palette-color images

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 24, 2024
@cagedmantis
Copy link
Contributor

It might be a good idea to tag the security team.

cc @golang/security

@cagedmantis cagedmantis changed the title x/image/tiff: Corrupt or malicious paletted images parse successfully and later panic in (*Palleted).At x/image/tiff: corrupt or malicious paletted images parse successfully and later panic in (*Palleted).At May 24, 2024
@cagedmantis cagedmantis added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 24, 2024
@neild
Copy link
Contributor

neild commented Jun 18, 2024

Thanks for the report. Assigning this CVE-2024-24792.

@jswright, how would you like to be attributed for the report?

@jswright
Copy link
Author

Sorry, missed this before now. I think John Wright <jsw@google.com> is the appropriate attribution. Also maybe worth noting that CVE-2023-36308 is really the same issue (this is the root cause).

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

No branches or pull requests

5 participants