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

ADA-ARGO-SA-01: help.chatUrl should be validated #9956

Closed
crenshaw-dev opened this issue Jul 12, 2022 · 3 comments · Fixed by #10417
Closed

ADA-ARGO-SA-01: help.chatUrl should be validated #9956

crenshaw-dev opened this issue Jul 12, 2022 · 3 comments · Fixed by #10417
Assignees
Labels
bug Something isn't working good first issue Good for newcomers security Security related

Comments

@crenshaw-dev
Copy link
Collaborator

It can currently be a javascript: link. Only admins can set this value, so it's reasonably safe. But it's probably better to URL validate that value to help admins avoid doing something dangerous.

@crenshaw-dev crenshaw-dev added bug Something isn't working security Security related labels Jul 12, 2022
@crenshaw-dev crenshaw-dev changed the title help.chatUrl should be sanitized help.chatUrl should be validated Jul 12, 2022
@todaywasawesome todaywasawesome added the good first issue Good for newcomers label Jul 13, 2022
@saumeya
Copy link
Contributor

saumeya commented Jul 14, 2022

I'd like to work on this, what does javascript: link mean @crenshaw-dev

@crenshaw-dev
Copy link
Collaborator Author

@saumeya, I believe the href attribute for the "chat" button can start with javascript: , meaning it can be used to run arbitrary JavaScript code instead of navigating to an external page. If I were a malicious Argo CD admin, I could embed code in that href to take malicious actions using the victim's credentials (do things like deleting an app).

The fix could make use of some of this validation code: 8bc3ef6

@crenshaw-dev
Copy link
Collaborator Author

Just to clarify: if someone is an Argo CD admin, there are a lot more malicious things they can do than this. So this isn't really a vulnerability. It would just be good to add an extra layer of protection. :-)

@crenshaw-dev crenshaw-dev changed the title help.chatUrl should be validated ADA-ARGO-SA-01: help.chatUrl should be validated Jul 15, 2022
crenshaw-dev added a commit that referenced this issue Feb 17, 2023
* fix: add url validation for help chat

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint check

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint fix

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* review comments

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

---------

Signed-off-by: saumeya <saumeyakatyal@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Feb 17, 2023
* fix: add url validation for help chat

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint check

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint fix

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* review comments

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

---------

Signed-off-by: saumeya <saumeyakatyal@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Feb 17, 2023
* fix: add url validation for help chat

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint check

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint fix

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* review comments

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

---------

Signed-off-by: saumeya <saumeyakatyal@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Feb 17, 2023
* fix: add url validation for help chat

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint check

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* lint fix

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

* review comments

Signed-off-by: saumeya <saumeyakatyal@gmail.com>

---------

Signed-off-by: saumeya <saumeyakatyal@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers security Security related
Projects
Development

Successfully merging a pull request may close this issue.

3 participants