hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

add a sanitizer that checks that the string does not start with "--"

Browse Source
This commit is contained in:
erik-krogh
2024-05-16 09:25:19 +02:00
parent b9a7f6a8f7
commit ea2b73bda2
3 changed files with 35 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
go/ql/lib/semmle/go/security/CommandInjectionCustomizations.qll
View File

@@ -52,4 +52,21 @@ module CommandInjection {
* A call to a regexp match function, considered as a barrier guard for command injection.
*/
class RegexpCheckBarrierAsSanitizer extends Sanitizer instanceof RegexpCheckBarrier { }
private predicate noDoubleDashPrefixCheck(DataFlow::Node hasPrefixNode, Expr e, boolean branch) {
exists(StringOps::HasPrefix hasPrefix | hasPrefix = hasPrefixNode |
e = hasPrefix.getBaseString().asExpr() and
hasPrefix.getSubstring().asExpr().getStringValue() = "--" and
branch = false
)
}
/**
* A call that confirms that the string does not start with `--`, considered as a barrier guard for command injection.
*/
class NoDoubleDashPrefixSanitizer extends Sanitizer {
NoDoubleDashPrefixSanitizer() {
this = DataFlow::BarrierGuard<noDoubleDashPrefixCheck/3>::getABarrierNode()
}
}
}

6
go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected
View File

@@ -14,6 +14,8 @@ edges
| GitSubcommands.go:10:13:10:27 | call to Query | GitSubcommands.go:14:30:14:36 | tainted | provenance | |
| GitSubcommands.go:10:13:10:27 | call to Query | GitSubcommands.go:15:35:15:41 | tainted | provenance | |
| GitSubcommands.go:10:13:10:27 | call to Query | GitSubcommands.go:16:36:16:42 | tainted | provenance | |
| GitSubcommands.go:32:13:32:19 | selection of URL | GitSubcommands.go:32:13:32:27 | call to Query | provenance | MaD:735 |
| GitSubcommands.go:32:13:32:27 | call to Query | GitSubcommands.go:37:32:37:38 | tainted | provenance | |
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL | SanitizingDoubleDash.go:9:13:9:27 | call to Query | provenance | MaD:735 |
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:13:25:13:31 | tainted | provenance | |
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:14:23:14:33 | slice expression | provenance | |
@@ -123,6 +125,9 @@ nodes
| GitSubcommands.go:14:30:14:36 | tainted | semmle.label | tainted |
| GitSubcommands.go:15:35:15:41 | tainted | semmle.label | tainted |
| GitSubcommands.go:16:36:16:42 | tainted | semmle.label | tainted |
| GitSubcommands.go:32:13:32:19 | selection of URL | semmle.label | selection of URL |
| GitSubcommands.go:32:13:32:27 | call to Query | semmle.label | call to Query |
| GitSubcommands.go:37:32:37:38 | tainted | semmle.label | tainted |
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL | semmle.label | selection of URL |
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | semmle.label | call to Query |
| SanitizingDoubleDash.go:13:15:13:32 | array literal [array] | semmle.label | array literal [array] |
@@ -212,6 +217,7 @@ subpaths
| GitSubcommands.go:14:30:14:36 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL | GitSubcommands.go:14:30:14:36 | tainted | This command depends on a $@. | GitSubcommands.go:10:13:10:19 | selection of URL | user-provided value |
| GitSubcommands.go:15:35:15:41 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL | GitSubcommands.go:15:35:15:41 | tainted | This command depends on a $@. | GitSubcommands.go:10:13:10:19 | selection of URL | user-provided value |
| GitSubcommands.go:16:36:16:42 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL | GitSubcommands.go:16:36:16:42 | tainted | This command depends on a $@. | GitSubcommands.go:10:13:10:19 | selection of URL | user-provided value |
| GitSubcommands.go:37:32:37:38 | tainted | GitSubcommands.go:32:13:32:19 | selection of URL | GitSubcommands.go:37:32:37:38 | tainted | This command depends on a $@. | GitSubcommands.go:32:13:32:19 | selection of URL | user-provided value |
| SanitizingDoubleDash.go:14:23:14:33 | slice expression | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | SanitizingDoubleDash.go:14:23:14:33 | slice expression | This command depends on a $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | user-provided value |
| SanitizingDoubleDash.go:40:23:40:30 | arrayLit | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | SanitizingDoubleDash.go:40:23:40:30 | arrayLit | This command depends on a $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | user-provided value |
| SanitizingDoubleDash.go:54:23:54:30 | arrayLit | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | SanitizingDoubleDash.go:54:23:54:30 | arrayLit | This command depends on a $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | user-provided value |

13
go/ql/test/query-tests/Security/CWE-078/GitSubcommands.go
View File

@@ -1,8 +1,8 @@
package main
import (
"net/http"
"os/exec"
"strings"
)
// BAD: using git subcommands that are vulnerable to arbitrary remote command execution
@@ -26,3 +26,14 @@ func gitSubcommandsGood(req *http.Request) {
exec.Command("git", "merge", tainted)
exec.Command("git", "add", tainted)
}
// BAD: using git subcommands that are vulnerable to arbitrary remote command execution
func gitSubcommandsGood2(req *http.Request) {
tainted := req.URL.Query()["cmd"][0]
if !strings.HasPrefix(tainted, "--") {
exec.Command("git", "clone", tainted) // GOOD, `tainted` cannot start with "--"
} else {
exec.Command("git", "clone", tainted) // BAD, `tainted` can start with "--"
}
}