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

Fix XSS via missing Binding syntax validation #34

Closed
wants to merge 11 commits into from
Closed

Fix XSS via missing Binding syntax validation #34

wants to merge 11 commits into from

Conversation

witekest
Copy link

@witekest witekest commented Oct 18, 2023 •
edited
Loading

Cross-site Scripting via missing Binding syntax validation vulnerability has been reported for the package github.com/crewjam/saml

https://nvd.nist.gov/vuln/detail/CVE-2023-45683

The vulnerability has been fixed upstream in the version 0.4.14 of the package.

GHSA-267v-3v32-g6q5

Grafana maintains own fork of the package in this repository. This pull request includes the cherry-pick from the upstream repository.

(cherry picked from commit crewjam/saml@b07b16c)

Jguer and others added 11 commits January 23, 2023 10:11
@Jguer @IevaVasiljeva @orgads
implement query signature verification
3b6b1ec 
add support for sha1 & sha512

add tests

use query sign in redirect

implement review feedback

- Return error if signature is unsupported
- wrap errors

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
Co-authored-by: Orgad Shaneh <orgads@gmail.com>
@alexanderzobnin
Fix reading compressed XML document
b75d97a
@alexanderzobnin
Don't fail if signature provided in query string, but not in the message
8197ae7
@alexanderzobnin
Only fail if no signature present at all
5f476db
@mattb18 @mgyongyosi
Update README example to use displayName attribute (crewjam#486)
87afeba
@crewjam @mgyongyosi
Merge pull request from GHSA-5mqj-xc49-246p
67cbfa0 
GHSA-5mqj-xc49-246p
@IevaVasiljeva
change how the list of namespaces that apply to an element is constru…
70af345 
…cted
@IevaVasiljeva
remove a comment
b301dc9
@IevaVasiljeva
add comments and a test
9900411
@mgyongyosi
Add cacheDuration, validUntil to RoleDescriptors (grafana#33)
94154cb
@crewjam @witekest
Merge pull request from GHSA-267v-3v32-g6q5
bad0489 
(cherry picked from commit b07b16c)
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@witekest witekest changed the title Merge pull request from GHSA-267v-3v32-g6q5 Fix XSS via missing Binding syntax validation Oct 19, 2023
@tolzhabayev tolzhabayev requested review from a team, linoman and alexanderzobnin and removed request for a team October 19, 2023 11:45
IevaVasiljeva
IevaVasiljeva approved these changes Oct 19, 2023
View reviewed changes
Copy link

@IevaVasiljeva IevaVasiljeva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Can you please accept the CLA so that we can merge this PR? More information is in this message: #34 (comment)

@witekest
Copy link
Author

I did accept the CLA but the check seems not to work for me.

@witekest
Copy link
Author

image

@mgyongyosi
Copy link

Hi @witekest, thank you for your contribution and for opening the PR. After some consideration we decided to sync with the crewjam/saml repo by rebasing our main on top of upstream/main to better follow the changes between the upstream and our forked repo. So unfortunately we need to close this PR, because these changes have already been added to the repo with the rebase.

Thanks again! 🙇

@mgyongyosi mgyongyosi closed this Oct 25, 2023
@witekest witekest deleted the cve-2023-45683 branch October 25, 2023 15:59
@witekest
Copy link
Author

Thanks as well

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

@IevaVasiljeva IevaVasiljeva IevaVasiljeva approved these changes

@linoman linoman Awaiting requested review from linoman linoman was automatically assigned from grafana/grafana-authnz-team

@alexanderzobnin alexanderzobnin Awaiting requested review from alexanderzobnin alexanderzobnin was automatically assigned from grafana/grafana-authnz-team

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@witekest @CLAassistant @mgyongyosi @IevaVasiljeva @Jguer @alexanderzobnin @mattb18 @crewjam