Improve Security on Form Actions #20116
Conversation
PR Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #20116 +/- ##
=======================================
Coverage 48.02% 48.02%
=======================================
Files 445 445
Lines 43892 43892
=======================================
Hits 21080 21080
Misses 22812 22812 ☔ View full report in Codecov by Sentry. |
|
How is this supposed to prevent XSS? |
the filter prevents malicious code to be injected from the browser URL to the jQuery selector arguments. |
|
@badbreze Can you explain how do we get XSS attack when using |
I that case jQuery should encode |
|
We need to start banning people, this is terrorism. Totally nonsense PR. Are you really still using Yii's client-side scripts, to the point where security is a concern? Really? Please, just waste your time elsewhere, preferably learning CS |
|
@Webkadabra Learning discussing without name calling and intimidation is possible and encouraged in Yii community. Please take note and correct that one. |
|
The thing is that we're passing arguments to jQuery directly from the URL, an extremely vulnerable door to the application code, so the PR is (at least in my case) a way to try securing this possible flaw, but i let you decide if this can be accepted or not. |
I get your point. tried to check and see how JQuery deals with encoding, I could not find decent docs. But if you want to propertly encode, may be you should look into native functions for that than simple regex replacement? Something like encodURI? |
|
This is not about encoding an URL, but about encoding a HTML attribute. I think we can safely assume that the most popular JS library can do such basic task. :) |
please point to the name calling you're so offended by |
Let me have little sleep, lol :) |
"Totally nonsense PR" |
this is factually a nonsense PR, too bad you're taking coding personally. You shouldv'e been trying to get good at coding, but you mastered getting offended |
|
@Webkadabra I suggest you to pick one of many open issues, create a PR to fix it and show us all how good coding looks like. So far all your comments were extremity unconstructive and probably wasted more people's time and mental energy than PRs you were commenting. |
I'm in shock by the random PRs of my favourite framework. I bet your wife may appreciate advice on what to do in life, but you are clearly out of order telling people what to do, assumig anything. Welcome to software developoment, you're not under your mom's protection anymore lol. I've only commented on PRs that suggest irrelevant code changes while also drawing a lot of attention. I'm sorry you have to lie publicly assuming MY comments took more time, haha. Well, if you're spending as much tought process replying in technical threads as you do with your weak attempt at starting a scandal, then sure - my comments probably wasted a lot of your time. But I see a ton of PRs with completely unnecessary, irrelevant and project-specifi changes to the framework code where people spend time explaining to guy like you that 2+2 is 4 and there is no need to say that it's not 3 |
|
@Webkadabra kindly take @rob006's advice and help us with your great skills iron bugs. It is welcome and appreciated. Again take note that you need to be respectful towards others here, even if you think you are the genius in the room! |
You tell me what to do, lie about name calling, then ask me to be respectful. Are you out of your mind? Who gave you the computer, did they not tell you to behave well? |
Oh sir with the advice, have you seen these braindead issues? A guy creates an infinite loop and claims it's Yii memory leak. Really, pick an issue, haha. The main issue is PR and issue spamming right now, with some villagers yelling god knows what |
|
@schmunk42 done. @Webkadabra if you want to be un-blocked, please contact me directly and be prepared to correct the way you communicate. |
| @@ -223,7 +223,7 @@ window.yii = (function ($) { | |||
| } | |||
| } else { | |||
| if (!isValidAction) { | |||
| action = pub.getCurrentUrl(); | |||
| action = pub.getSafeUrl(); | |||
| } | |||
| $form = $('<form/>', {method: method, action: action}); | |||
There was a problem hiding this comment.
Hmm... jQuery doesn't encode attributes?
This PR is intended to secure the Action of the Forms by cleaning up malicious codes from the provider URL as much as possible to avoid XSS