Conversation

AdamKorcz

Adds a length check to avoid an out-of-range panic.

@coveralls

Coverage Status

coverage: 88.586% (-0.2%) from 88.765%
when pulling f4db242 on AdamKorcz:fix-out-of-range
into 53694f8 on go-playground:master.

@AdamKorczAdamKorcz force-pushed the fix-out-of-range branch 3 times, most recently from 5554ed1 to 316ebee Compare November 18, 2023 15:26
@davidhadas

@robinlieb, this PR relates to a Security Audit done to the CNCF Knative Project,
Knative uses this repository as a dependency and the audit requires this to be fixed.

Can you check to see if this PR is expected to be fixed any time soon, or should Knative look for alternatives instead?

@robinlieb

Hi @AdamKorcz,
is this change still needed after the merge of #173?

@AdamKorcz

Hi @AdamKorcz, is this change still needed after the merge of #173?

Yes

@AdamKorcz

@robinlieb Can you have a look at https://.com/go-playground/webhooks/security/advisories/GHSA-m7vc-6h95-xrjq and invite @davidhadas as a collaborator there as well?

@evankanderson

@robinlieb Can you have a look at https://.com/go-playground/webhooks/security/advisories/GHSA-m7vc-6h95-xrjq and invite @davidhadas as a collaborator there as well?

I'm the other security lead on Knative, and I'd appreciate if I could be on this issue as well.

@evankanderson

Actually, #173 did fix this by swapping []byte(signature[5:]) for strings.TrimSignature, which removes the slice indexing.

I think we can close this given #173 being merged. Do you agree @AdamKorcz ?

@robinlieb

Seems like I don't have access neither. @deankarn can you have a look and add me and the two mentioned in the thread to this security advisory?

@deankarn

Maybe I missed something how would I add someone to the security advisory? Not sure I understand.

@AdamKorcz

Maybe I missed something how would I add someone to the security advisory? Not sure I understand.

https://docs..com/en/code-security/security-advisories/working-with-repository-security-advisories/adding-a-collaborator-to-a-repository-security-advisory

@deankarn

OK y'all should be added now as collaborators.

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

Successfully merging this pull request may close these issues.