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

Go support for GDPR v1.12.0 crashes with Go Fuzz test #40

Open
ntframeplay opened this issue Jan 9, 2024 · 7 comments
Open

Go support for GDPR v1.12.0 crashes with Go Fuzz test #40

ntframeplay opened this issue Jan 9, 2024 · 7 comments
Assignees

Comments

@ntframeplay
Copy link

Using the fuzz test string below (or similar)

go test fuzz v1
string("C0000000000000000000000000000000000AAdA000000000000")

the following crash & stack trace occurs:

image

Fuzz test func:

func FuzzTCF(f *testing.F) {
	// Only one function to be tested
	f.Fuzz(func(t *testing.T, tcfStr string) {
		_, _ = tcf(tcfStr)
	})
}

and tcf func

func tcf(tcf_str string) (tcfData, error) {
	if len(tcf_str) == 0 {
		return tcfData{}, nil
	}
	consent, err := vendorconsent.ParseString(tcf_str) // string is base64 encoded
	if err != nil {
		return tcfData{}, err
	}

	return tcfData{
		tcf_str,
		consent.VendorListVersion(),
		consent.VendorConsent(uint16(TCFFramePlayVendorID)),
		consent.PurposeAllowed(3),
	}, nil
}

index out of range occurs in return value of
/go-gdpr/vendorconsent/tcf2.isSet(...)

@SyntaxNode
Copy link
Collaborator

SyntaxNode commented Jan 26, 2024

I don't doubt the fuzz testing identified an edge case, but I'm unable to reproduce this error. I ran the fuzz test on Go 1.20.5 assuming TCFFramePlayVendorID = 970 and using the seed value of C0000000000000000000000000000000000AAdA000000000000.

The TCF consent string is invalid and for me, ParseString returns an invalid consent data: no legitimate interest start position error. It doesn't get to the code which panics.

Could you please help in identifying a specific consent string to demonstrate the index out of range panic?

@ntframeplay
Copy link
Author

ntframeplay commented Feb 7, 2024

OK, sorry for not getting back to you earlier. I will try a number of different consent strings and record findings

@aradilov
Copy link

aradilov commented Feb 15, 2024

I confirm the issue.
Have the same fatal with consent C0000000000000000000000000000000000AAdA000000000000

go version go1.22.0 darwin/amd64

GDPR v1.12.0

tcf_str := "C0000000000000000000000000000000000AAdA000000000000"
consent, err := vendorconsent.ParseString(tcf_str)

github.com/prebid/go-gdpr/vendorconsent/tcf2.isSet(...)
        /github.com/prebid/go-gdpr/vendorconsent/tcf2/metadata.go:212
github.com/prebid/go-gdpr/vendorconsent/tcf2.Parse({0xc000132040, 0x26, 0x26})
        /github.com/prebid/go-gdpr/vendorconsent/tcf2/consent.go:68 +0x53b
github.com/prebid/go-gdpr/vendorconsent/tcf2.ParseString({0x207402c, 0x33})
       /github.com/prebid/go-gdpr/vendorconsent/tcf2/consent.go:35 +0xc8

@bsardo bsardo assigned hhhjort and unassigned SyntaxNode Feb 27, 2024
@hhhjort
Copy link
Collaborator

hhhjort commented Feb 27, 2024

I see at least one bug now. consent.go:68 We check bit to flag that the following is a bit field or a range encoding. The IsSet() function does not verify that the consent string is long enough to actually contain the bit it is looking for. Just above we ParseUInt16() which will insure that the 16 bits in front of this bit exist. So we will only run into an error if the MaxVendorId for the legitimate interest section falls exactly on a byte boundary, and the consent string ends with MaxVendorId. This is a rare case, which explains why it has evaded detection for so long.

There is another IsSet() call above on line 52, for bit 229, but parseMetadata() above that will insure that this bit exists in the consent string. There does not seem to be any other unprotected IsSet calls, so the best fix is probably a range check in front of line 68 to ensure that the bit exists.

@bsardo
Copy link
Contributor

bsardo commented Feb 28, 2024

@ntframeplay are you interested in pushing up a fix?

@ntframeplay
Copy link
Author

Sorry, at this stage I dont have the bandwidth.

@SyntaxNode
Copy link
Collaborator

Sorry, at this stage I dont have the bandwidth.

No worries. Thank you for identifying this bug. We'll schedule it within our dev team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants