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

optionally allow signed cookies, escape token injected into html #14

Merged
merged 1 commit into from Oct 8, 2014

Conversation

mtths
Copy link
Contributor

@mtths mtths commented Oct 8, 2014

still needs tests, trying to wrap my head around that now

escape token injected into html
@mtths
Copy link
Contributor Author

mtths commented Oct 8, 2014

didn't quite get the hang of the tests yet, but upon reviewing the patch i found it more sensible to uri_escape the token instead of entity-encoding it (line 161). what do you think?

chizmw added a commit that referenced this pull request Oct 8, 2014
optionally allow signed cookies, escape token injected into html
@chizmw chizmw merged commit 6e3172a into chizmw:master Oct 8, 2014
@chizmw
Copy link
Owner

chizmw commented Oct 8, 2014

Oops, I didn't see your extra comment before merging the pull request.
It did take me quite some time to work out how to even begin testing this module. It's not as good as it could be, but it's passable.

@mtths
Copy link
Contributor Author

mtths commented Oct 8, 2014

mostly against MITM attacks, where the attacker modifies the cookie and the submitted token.
e.g. attacker resets token and cookie as

"><svg/onload=prompt(document.domain)>

without the patch that triggers xss in the resulting html, and the cookie validates just fine.
with the patch (and secret set to a defined value) the tampering is detected and "invalid signature" is served

@chizmw
Copy link
Owner

chizmw commented Oct 8, 2014

Oh, yes. Very nice!
I think that HTML::Escape::escape_html() is the right choice here, no?
It appears to do what's needed, seems reasonably up to date, and appears to be at least "fast enough".

chizmw added a commit that referenced this pull request Oct 13, 2014
chizmw added a commit that referenced this pull request Oct 13, 2014
 - Optionally allow signed cookies escape token injected into html (pull #14)
chizmw added a commit that referenced this pull request Oct 13, 2014
 - Optionally allow signed cookies escape token injected into html (pull #14)
@chizmw
Copy link
Owner

chizmw commented Oct 13, 2014

Just pushed to PAUSE as 0.0.9. Should filter out over the next few hours.

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.

None yet

2 participants