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

Further fixes for grave character security protection #4356

Closed
ddb4github opened this issue Jul 22, 2021 · 11 comments
Closed

Further fixes for grave character security protection #4356

ddb4github opened this issue Jul 22, 2021 · 11 comments
Labels
duplicate Duplicate of another issue resolved A fixed issue SECURITY A security issue reported through CVE
Milestone

Comments

@ddb4github
Copy link
Contributor

Describe the bug

  • cacti/data_templates.php:
    Name column does not escape grave(`) char.
    Example:
    Cacti Stats - Export Duration	onmouseover=`alert(188)`
    
    Ref: https://davidmurdoch.com/2017/09/02/the-grave-accent-and-xss/
  • graph_templates.php?action=template_edit&id=123
    Graph Item Inputs-->Name column, cruly braces(})
    Example:
    a onmouseover=55+{toString:alert}//
    

To Reproduce

Hardly reproduce under Firefox/Chrome

@ddb4github ddb4github added bug Undesired behaviour unverified Some days we don't have a clue labels Jul 22, 2021
@netniV
Copy link
Member

netniV commented Jul 22, 2021

Does this affect any other areas?

@ddb4github
Copy link
Contributor Author

  • data_templates.php issue is about function filter_value
  • graph_templates.php issue is about function html_escape
  • current filter_value does not use html_escape

@TheWitness
Copy link
Member

@ddb4github, didn't I propose and didn't we implement a str_replace() on the graves character?

@TheWitness
Copy link
Member

This is already in the 1.2.x branch. Is it nor right?

image

@netniV
Copy link
Member

netniV commented Sep 25, 2021

If you look more closely though, I think he's saying that the filter_value calls we have don't use html_escape so if any of the filter_values are reused in display, they would allow XSS.

@netniV
Copy link
Member

netniV commented Sep 25, 2021

I was unable to reproduce this myself, so I have basically applied the same code in filter_value as was present in html_escape. This will make the filters align better as well so it should be a win win scenario.

@netniV netniV added resolved A fixed issue SECURITY A security issue reported through CVE unverified Some days we don't have a clue and removed bug Undesired behaviour unverified Some days we don't have a clue labels Sep 25, 2021
@netniV netniV added this to the 1.2.19 milestone Sep 25, 2021
@TheWitness
Copy link
Member

That's cause we fixed it already ;)

@TheWitness
Copy link
Member

Here is the commit that fixed the graves issue.

8b42b65

Going to close this as duplicate.

@TheWitness TheWitness added duplicate Duplicate of another issue and removed unverified Some days we don't have a clue labels Sep 25, 2021
@netniV
Copy link
Member

netniV commented Sep 26, 2021

This isn't a duplicate. It's applying the fix to the filter_value() function.

@TheWitness
Copy link
Member

Yea, I noted that.

@netniV
Copy link
Member

netniV commented Sep 26, 2021

Confused as you said it was a duplicate.

@netniV netniV changed the title XSS issue with special char: grave and curly brace Further fixes for grave character security protection Oct 3, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
duplicate Duplicate of another issue resolved A fixed issue SECURITY A security issue reported through CVE
Projects
None yet
Development

No branches or pull requests

3 participants