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

Use proper jQuery objects instead of strings for better security #630

Merged
merged 2 commits into from
Sep 19, 2020
Merged

Use proper jQuery objects instead of strings for better security #630

merged 2 commits into from
Sep 19, 2020

Conversation

m417z
Copy link
Contributor

@m417z m417z commented Aug 2, 2020

Fixes #629, improves overall security and robustness, and readability too.
I didn't test it thoroughly, but the bundled example works properly.

Note that while many places prone to XSS are now safe, there are still places which look dangerous. For example:
https://github.com/m417z/yadcf/blob/cd4bf396bc7cc46aa0f7456d5ab0775ad9a94525/jquery.dataTables.yadcf.js#L1559

If the string which is part of the expression is untrusted, it must be escaped.

This pull request took more time than I expected, I'd appreciate if it gets merged.
Thanks for the library.

@vedmack
Copy link
Owner

vedmack commented Aug 3, 2020

Wow, thanks @m417z will merge it in the upcoming days

@m417z
Copy link
Contributor Author

m417z commented Aug 3, 2020

Thanks. I pushed another commit which actually fixes #629, not only security-wise, even if you use a string like:
A"B'C&nbsp;<script>alert(1)</script>.

@vedmack vedmack merged commit 7aefdbf into vedmack:master Sep 19, 2020
@m417z m417z deleted the use-jquery-objects branch September 19, 2020 11:30
@vedmack
Copy link
Owner

vedmack commented Sep 19, 2020

Ohh noooooo!
Only now I noticed that you did you PR on the stable 0.9.3 version instead of the latest beta version of 0.9.4 that is located in yadcf/src/

@m417z
Copy link
Contributor Author

m417z commented Sep 19, 2020

Heck. I created a rebased version, and found some potential bugs that I'll mention in the new PR:
#634

vedmack added a commit that referenced this pull request Sep 26, 2020
PR (by m417z) Use proper jQuery objects instead of strings for better security - #630

a bit of code clean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling untrusted input (avoiding XSS)
2 participants