-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Ruby: Add rb/insecure-dependency query #8598
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
37cedda
Ruby: Add InsecureDependencyResolution query
hmac d13bbba
Ruby: Add change note for rb/insecure-dependency
hmac 167bda2
Ruby: Add QLDoc for InsecureDependencyQuery.qll
hmac 3d96c5e
Ruby: Add test case for rb/insecure-dependency
hmac 5814db1
Ruby: Fix bug in rb/insecure-dependency query
hmac ae60d40
Ruby: Fix typo in rb/insecure-dependency qhelp
hmac 8f3578c
Ruby: Include query results in test
hmac File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
91 changes: 91 additions & 0 deletions
91
ruby/ql/lib/codeql/ruby/security/InsecureDependencyQuery.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/** | ||
* Provides predicates for reasoning about insecure dependency configurations. | ||
*/ | ||
|
||
private import ruby | ||
|
||
/** | ||
* A method call in a Gemfile. | ||
*/ | ||
private class GemfileMethodCall extends MethodCall { | ||
GemfileMethodCall() { this.getLocation().getFile().getBaseName() = "Gemfile" } | ||
} | ||
|
||
/** | ||
* Method calls that configure gem dependencies and can specify (possibly insecure) URLs. | ||
*/ | ||
abstract private class RelevantGemCall extends GemfileMethodCall { | ||
abstract Expr getAUrlPart(); | ||
} | ||
|
||
/** | ||
* A call to `source`. | ||
*/ | ||
private class SourceCall extends RelevantGemCall { | ||
SourceCall() { this.getMethodName() = "source" } | ||
|
||
override Expr getAUrlPart() { result = this.getAnArgument() } | ||
} | ||
|
||
/** | ||
* A call to `git_source`. | ||
*/ | ||
private class GitSourceCall extends RelevantGemCall { | ||
GitSourceCall() { this.getMethodName() = "git_source" } | ||
|
||
override Expr getAUrlPart() { result = this.getBlock().getLastStmt() } | ||
} | ||
|
||
/** | ||
* A call to `gem`. | ||
*/ | ||
private class GemCall extends RelevantGemCall { | ||
GemCall() { this.getMethodName() = "gem" } | ||
|
||
override Expr getAUrlPart() { result = this.getKeywordArgument(["source", "git"]) } | ||
} | ||
|
||
/** | ||
* Holds if `s` is a URL with an insecure protocol. `proto` is the protocol. | ||
*/ | ||
bindingset[s] | ||
private predicate hasInsecureProtocol(string s, string proto) { | ||
proto = s.regexpCapture("^(http|ftp):.+", 1).toUpperCase() | ||
} | ||
|
||
/** | ||
* Holds if `e` is a string containing a URL that uses the insecure protocol `proto`. | ||
*/ | ||
private predicate containsInsecureUrl(Expr e, string proto) { | ||
// Handle cases where the string as a whole has no constant value (due to interpolations) | ||
// but has a known prefix. E.g. "http://#{foo}" | ||
exists(StringComponent c | c = e.(StringlikeLiteral).getComponent(0) | | ||
hasInsecureProtocol(c.getConstantValue().getString(), proto) | ||
) | ||
or | ||
hasInsecureProtocol(e.getConstantValue().getString(), proto) | ||
} | ||
|
||
/** | ||
* Returns the suggested protocol to use in place of the insecure protocol `proto`. | ||
*/ | ||
bindingset[proto] | ||
private string suggestedProtocol(string proto) { | ||
proto = "HTTP" and result = "HTTPS" | ||
or | ||
proto = "FTP" and result = "FTPS or SFTP" | ||
} | ||
|
||
/** | ||
* Holds if `url` is a string containing a URL that uses an insecure protocol. | ||
* `msg` is the alert message that will be displayed to the user. | ||
*/ | ||
predicate insecureDependencyUrl(Expr url, string msg) { | ||
exists(RelevantGemCall call, string proto | | ||
url = call.getAUrlPart() and | ||
containsInsecureUrl(url, proto) and | ||
msg = | ||
"Dependency source URL uses the unencrypted protocol " + proto + ". Use " + | ||
suggestedProtocol(proto) + " instead." | ||
) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
--- | ||
category: newQuery | ||
--- | ||
* Added a new query, `rb/insecure-dependency`. The query finds cases where Ruby gems may be downloaded over an insecure communication channel. |
54 changes: 54 additions & 0 deletions
54
ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.qhelp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
<!DOCTYPE qhelp PUBLIC | ||
"-//Semmle//qhelp//EN" | ||
"qhelp.dtd"> | ||
<qhelp> | ||
|
||
<overview> | ||
<p> | ||
Using an insecure protocol like HTTP or FTP to download dependencies makes the build process vulnerable to a | ||
man-in-the-middle (MITM) attack. | ||
</p> | ||
<p> | ||
This can allow attackers to inject malicious code into the downloaded dependencies, and thereby | ||
infect the build artifacts and execute arbitrary code on the machine building the artifacts. | ||
</p> | ||
|
||
</overview> | ||
<recommendation> | ||
|
||
<p>Always use a secure protocol, such as HTTPS or SFTP, when downloading artifacts from a URL.</p> | ||
|
||
</recommendation> | ||
|
||
<example> | ||
<p> | ||
The below example shows a <code>Gemfile</code> that specifies a gem source using the insecure HTTP protocol. | ||
</p> | ||
<sample src="examples/bad_gemfile.rb" /> | ||
<p> | ||
The fix is to change the protocol to HTTPS. | ||
</p> | ||
<sample src="examples/good_gemfile.rb" /> | ||
</example> | ||
|
||
<references> | ||
<li> | ||
Jonathan Leitschuh: | ||
<a href="https://infosecwriteups.com/want-to-take-over-the-java-ecosystem-all-you-need-is-a-mitm-1fc329d898fb"> | ||
Want to take over the Java ecosystem? All you need is a MITM! | ||
</a> | ||
</li> | ||
<li> | ||
Max Veytsman: | ||
<a href="https://max.computer/blog/how-to-take-over-the-computer-of-any-java-or-clojure-or-scala-developer/"> | ||
How to take over the computer of any Java (or Clojure or Scala) Developer. | ||
</a> | ||
</li> | ||
<li> | ||
Wikipedia: <a href="https://en.wikipedia.org/wiki/Supply_chain_attack">Supply chain attack.</a> | ||
</li> | ||
<li> | ||
Wikipedia: <a href="https://en.wikipedia.org/wiki/Man-in-the-middle_attack">Man-in-the-middle attack.</a> | ||
</li> | ||
</references> | ||
</qhelp> |
22 changes: 22 additions & 0 deletions
22
ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/** | ||
* @name Dependency download using unencrypted communication channel | ||
* @description Using unencrypted protocols to fetch dependencies can leave an application | ||
* open to man-in-the-middle attacks. | ||
* @kind problem | ||
* @problem.severity warning | ||
* @security-severity 8.1 | ||
* @precision high | ||
* @id rb/insecure-dependency | ||
* @tags security | ||
* external/cwe/cwe-300 | ||
* external/cwe/cwe-319 | ||
* external/cwe/cwe-494 | ||
* external/cwe/cwe-829 | ||
*/ | ||
|
||
import ruby | ||
import codeql.ruby.security.InsecureDependencyQuery | ||
|
||
from Expr url, string msg | ||
where insecureDependencyUrl(url, msg) | ||
select url, msg |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
source "http://rubygems.org" | ||
|
||
gem "my-gem-a", "1.2.3" |
3 changes: 3 additions & 0 deletions
3
ruby/ql/src/queries/security/cwe-300/examples/good_gemfile.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
source "https://rubygems.org" | ||
|
||
gem "my-gem-a", "1.2.3" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
source "https://rubygems.org" # GOOD | ||
source "http://rubygems.org" # $result=BAD | ||
source "ftp://rubygems.org" # $result=BAD | ||
source "ftps://rubygems.org" # GOOD | ||
source "unknown://rubygems.org" # GOOD | ||
|
||
git_source(:a) { "https://github.com" } # GOOD | ||
git_source(:b) { "http://github.com" } # $result=BAD | ||
git_source(:c) { "ftp://github.com" } # $result=BAD | ||
git_source(:d) { "ftps://github.com" } # GOOD | ||
git_source(:e) { "unknown://github.com" } # GOOD | ||
|
||
git_source(:f) { |name| "https://github.com/#{name}" } # GOOD | ||
git_source(:g) { |name| "http://github.com/#{name}" } # $result=BAD | ||
git_source(:h) { |name| "ftp://github.com/#{name}" } # $result=BAD | ||
git_source(:i) { |name| "ftps://github.com/#{name}" } # GOOD | ||
git_source(:j) { |name| "unknown://github.com/#{name}" } # GOOD | ||
|
||
git_source(:k) do |name| | ||
foo | ||
"https://github.com/#{name}" } # GOOD | ||
end | ||
git_source(:l) do |name| | ||
foo | ||
"http://github.com/#{name}" } # $result=BAD | ||
end | ||
git_source(:m) do |name| | ||
foo | ||
"ftp://github.com/#{name}" } # $result=BAD | ||
end | ||
git_source(:n) do |name| | ||
foo | ||
"ftps://github.com/#{name}" } # GOOD | ||
end | ||
git_source(:o) do |name| | ||
foo | ||
"unknown://github.com/#{name}" } # GOOD | ||
end | ||
|
||
gem "jwt", "1.2.3", git: "https://github.com/jwt/ruby-jwt" # GOOD | ||
gem "jwt", "1.2.3", git: "http://github.com/jwt/ruby-jwt" # $result=BAD | ||
gem "jwt", "1.2.3", git: "ftp://github.com/jwt/ruby-jwt" # $result=BAD | ||
gem "jwt", "1.2.3", git: "ftps://github.com/jwt/ruby-jwt" # GOOD | ||
gem "jwt", "1.2.3", git: "unknown://github.com/jwt/ruby-jwt" # GOOD | ||
|
||
gem "jwt", "1.2.3", :git => "https://github.com/jwt/ruby-jwt" # GOOD | ||
gem "jwt", "1.2.3", :git => "http://github.com/jwt/ruby-jwt" # $result=BAD | ||
gem "jwt", "1.2.3", :git => "ftp://github.com/jwt/ruby-jwt" # $result=BAD | ||
gem "jwt", "1.2.3", :git => "ftps://github.com/jwt/ruby-jwt" # GOOD | ||
gem "jwt", "1.2.3", :git => "unknown://github.com/jwt/ruby-jwt" # GOOD | ||
|
||
gem "jwt", "1.2.3", source: "https://rubygems.org" # GOOD | ||
gem "jwt", "1.2.3", source: "http://rubygems.org" # $result=BAD | ||
gem "jwt", "1.2.3", source: "ftp://rubygems.org" # $result=BAD | ||
gem "jwt", "1.2.3", source: "ftps://rubygems.org" # GOOD | ||
gem "jwt", "1.2.3", source: "unknown://rubygems.org" # GOOD |
16 changes: 16 additions & 0 deletions
16
ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
failures | ||
#select | ||
| Gemfile:2:8:2:28 | "http://rubygems.org" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. | | ||
| Gemfile:3:8:3:27 | "ftp://rubygems.org" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. | | ||
| Gemfile:8:18:8:36 | "http://github.com" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. | | ||
| Gemfile:9:18:9:35 | "ftp://github.com" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. | | ||
| Gemfile:14:25:14:51 | "http://github.com/#{...}" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. | | ||
| Gemfile:15:25:15:50 | "ftp://github.com/#{...}" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. | | ||
| Gemfile:25:5:25:31 | "http://github.com/#{...}" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. | | ||
| Gemfile:29:5:29:30 | "ftp://github.com/#{...}" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. | | ||
| Gemfile:41:26:41:57 | "http://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. | | ||
| Gemfile:42:26:42:56 | "ftp://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. | | ||
| Gemfile:47:29:47:60 | "http://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. | | ||
| Gemfile:48:29:48:59 | "ftp://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. | | ||
| Gemfile:53:29:53:49 | "http://rubygems.org" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. | | ||
| Gemfile:54:29:54:48 | "ftp://rubygems.org" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. | |
23 changes: 23 additions & 0 deletions
23
ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import ruby | ||
import TestUtilities.InlineExpectationsTest | ||
import codeql.ruby.security.InsecureDependencyQuery | ||
|
||
class InsecureDependencyTest extends InlineExpectationsTest { | ||
InsecureDependencyTest() { this = "InsecureDependencyTest" } | ||
|
||
override string getARelevantTag() { result = "BAD" } | ||
|
||
override predicate hasActualResult(Location location, string element, string tag, string value) { | ||
tag = "result" and | ||
value = "BAD" and | ||
exists(Expr e | | ||
insecureDependencyUrl(e, _) and | ||
location = e.getLocation() and | ||
element = e.toString() | ||
) | ||
} | ||
} | ||
|
||
from Expr url, string msg | ||
where insecureDependencyUrl(url, msg) | ||
select url, msg |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# Calls to `gem` etc. outside of the Gemfile should be ignored, since they may not be configuring dependencies. | ||
|
||
gem "foo", git: "http://foo.com" | ||
git_source :a { |x| "http://foo.com" } | ||
source "http://foo.com" |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.