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

Update login.ztml #106

Merged
merged 1 commit into from
Feb 10, 2014
Merged

Update login.ztml #106

merged 1 commit into from
Feb 10, 2014

Conversation

japp-0xlabs
Copy link
Contributor

XSS fixes.

If form action is empty, the target is self script. PHP_SELF not needed.

XSS fixes.

If form action is empty, the target is self script. PHP_SELF not needed.
@allebb
Copy link
Contributor

allebb commented Feb 10, 2014

This was previously added to ensure legacy browser support but I guess as the latest theme is using a whole load of jQuery and HTML5 I guess we could remove the attribute value for no other reason but to remove a few characters from the login template.

allebb added a commit that referenced this pull request Feb 10, 2014
Removal of FORM POST URL (use of PHP_SELF constant) as not required (code clean-up).
@allebb allebb merged commit e24b0f7 into zpanel:master Feb 10, 2014
@japp-0xlabs
Copy link
Contributor Author

I think other little reason could be XSS patch as I said, if you don't sanitize PHP_SELF and you try something like /index.php/%22%3E%3Cscript%3Ealert%28document.cookie%29%3C%2fscript%3E you will get nice javascript alert.

@5050
Copy link
Contributor

5050 commented Feb 10, 2014

(only for note) Since HTML5, action="" is reported as an error by html validator (http://validator.w3.org/). when empty, the directive "action" should be completely removed.

@allebb
Copy link
Contributor

allebb commented Feb 10, 2014

Cheers 5050, I'll remove the attribute altogether 👍

@allebb
Copy link
Contributor

allebb commented Feb 10, 2014

Fixed: a8a540d - Good call there 5050!

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.

3 participants