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

Fix sole_raw_text to allow $ #8

Merged
merged 1 commit into from
Oct 18, 2017
Merged

Fix sole_raw_text to allow $ #8

merged 1 commit into from
Oct 18, 2017

Conversation

cubeguerrero
Copy link
Contributor

Changes

Changes the regex expression:
from: ~r/^[^\n$]+/
to: ~r/^[^\n]+$/

The former matches the $ literally, while the new change will asserts position at the end of the string, or before the line terminator right at the end of the string (if any)

Issues

Fixes #7

@skull-squadron
Copy link

Maintainers and owner should review OWASP regex best practices to avoid unintentionally adding a CVE-worthy flaw here or elsewhere, because $ in a character set is not a non-terminal. Also, prefer \A and \z unless input is trusted and there a specific need to match only on one line.

https://github.com/attackercan/regexp-security-cheatsheet

https://www.owasp.org/index.php/Regular_Expression_Security_Cheatsheet

@rstacruz rstacruz merged commit e806ee7 into rstacruz:master Oct 18, 2017
@rstacruz
Copy link
Owner

Sorry this took a while, I'll be releasing this as 0.9.0 soon.

@rstacruz
Copy link
Owner

Also, thanks for the regexp advice @steakknife! AFAIK we should be good with $ in here.

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