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

Code scanning alert on escapeHash #341

Closed
3 tasks done
RamonvdW opened this issue Mar 6, 2023 · 4 comments
Closed
3 tasks done

Code scanning alert on escapeHash #341

RamonvdW opened this issue Mar 6, 2023 · 4 comments
Labels

Comments

@RamonvdW
Copy link

RamonvdW commented Mar 6, 2023

Before submitting...

Context

/js/global.js contains the function escapeHash()
I enabled Code Scanning with CodeQL in my GitHub project and the following came up:

Incomplete string escaping or encoding
return hash.replace(/(:|\.|\[|\]|,|=|\/)/g, '\\$1');
This does not escape backslash characters in the input.

References: CWE-20, CWE-80 and CWE-116

Current Behavior

No response

Expected behavior

Should also escape backslash characters in the input

Possible Solutions or Causes

Is it a false positive or a real warning?

Your Environment

  • Version used: 1.1
  • Browser Name and version: n/a
  • Operating System and version (desktop or mobile): n/a
  • Additional information you want to tell us: GitHub code scanning with CodeQL
@wuda-io
Copy link
Member

wuda-io commented Mar 6, 2023

Hello @RamonvdW
thanks for pointing that out. I checked where it is used and found out that it is only used by Tabs.
I dont know why it is needed, probably to prevent XSS.

Thats interesting... I saw that a Tab-Content Element is added via JQuery Selector and it takes as input the href-hash of the links,
escapes it and then then uses the jquery selector with that and places it as Element in the Tab-Content-Element.

Possible solution:
Selector part with escaping should be removed and only show or hide the tab-contents,
also it is faster because the dom-html is preloaded.

@wuda-io wuda-io added bug Something isn't working security and removed bug Something isn't working labels Mar 6, 2023
@wuda-io
Copy link
Member

wuda-io commented Mar 6, 2023

I looked into it and it should be no security concern...

@wuda-io
Copy link
Member

wuda-io commented Mar 6, 2023

What can happen is that untrusted data can injected into the cash-selector via location-hash or link-hash.
I dont know if there are any vulnerabilites in the cash library.
Maybe here some interesting link: https://blog.mindedsecurity.com/2011/07/jquery-is-sink.html
Solution for me would be to remove jquery.
I dont know if the native querySelector Function has code injection issues but i think not.
At least I did not find much about it online.

@wuda-io
Copy link
Member

wuda-io commented Apr 1, 2023

no security concern.
also improved in v2-dev

@wuda-io wuda-io closed this as completed Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants