-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Ruby: Add MissingRegExpAnchor query #8573
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
Conversation
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression TODO: describe the danger of using line anchors like References
|
1 similar comment
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression TODO: describe the danger of using line anchors like References
|
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression TODO: describe the danger of using line anchors like References
|
5edf926
to
6535c7d
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression TODO: describe the danger of using line anchors like References
|
6535c7d
to
b7a5bc4
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression TODO: describe the danger of using line anchors like References
|
b7a5bc4
to
f3a5b41
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression TODO: describe the danger of using line anchors like References
|
1 similar comment
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression TODO: describe the danger of using line anchors like References
|
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression In Ruby the anchors References
|
QHelp previews: ruby/ql/src/queries/security/cwe-020/MissingRegExpAnchor.qhelpMissing regular expression anchorSanitizing untrusted input with regular expressions is a common technique. However, it is error-prone to match untrusted input against regular expressions without anchors such as Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches. RecommendationUse anchors to ensure that regular expressions match at the expected locations. ExampleThe following example code checks that a URL redirection will reach the class UsersController < ActionController::Base
def index
# BAD: the host of `params[:url]` may be controlled by an attacker
if params[:url].match? /https?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end The check with the regular expression match is, however, easy to bypass. For example by embedding class UsersController < ActionController::Base
def index
# GOOD: the host of `params[:url]` can not be controlled by an attacker
if params[:url].match? /\Ahttps?:\/\/www\.example\.com\//
redirect_to params[:url]
end
end
end A related mistake is to write a regular expression with multiple alternatives, but to only include an anchor for one of the alternatives. As an example, the regular expression In Ruby the anchors References
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I like the addition of checks for ^
/$
rather than \A
/\z
.
There's an unrelated build failure in the DCA run that may be fixed by a rebase.
5dffc67
to
bbc3043
Compare
It finally succeeded! It feels like I've been attempting DCA runs on this PR since the dawn of time... The vast majority of results are in tests, but I've checked a few of the non-test ones and they look legitimate. |
This is a port of the JS query of the same name. A few adjustments are needed to adapt to our dataflow and regex libraries, so it's not so easy to share the implementation. I've also added a Ruby-specific check for regexes that use
^
and$
, which match the start/end of a line rather than the whole string. These regexes should probably use\A
and\z
instead.