Skip to content

Commit 040d971

Browse files
sabrowning1sabrowning1
authored
Merge branch 'main' into sabrowning1/queries-panel-language-selector
2 parents e95bfc8 + c14d404 commit 040d971

File tree

46 files changed

+2134
-291
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+2134
-291
lines changed

‎cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,16 @@
11
edges
2-
subpaths
2+
| main.cpp:6:27:6:30 | argv indirection | main.cpp:10:20:10:23 | argv indirection |
3+
| main.cpp:10:20:10:23 | argv indirection | tests.cpp:618:32:618:35 | argv indirection |
4+
| tests.cpp:613:19:613:24 | source indirection | tests.cpp:615:17:615:22 | source indirection |
5+
| tests.cpp:618:32:618:35 | argv indirection | tests.cpp:643:9:643:15 | access to array indirection |
6+
| tests.cpp:643:9:643:15 | access to array indirection | tests.cpp:613:19:613:24 | source indirection |
37
nodes
8+
| main.cpp:6:27:6:30 | argv indirection | semmle.label | argv indirection |
9+
| main.cpp:10:20:10:23 | argv indirection | semmle.label | argv indirection |
10+
| tests.cpp:613:19:613:24 | source indirection | semmle.label | source indirection |
11+
| tests.cpp:615:17:615:22 | source indirection | semmle.label | source indirection |
12+
| tests.cpp:618:32:618:35 | argv indirection | semmle.label | argv indirection |
13+
| tests.cpp:643:9:643:15 | access to array indirection | semmle.label | access to array indirection |
14+
subpaths
415
#select
16+
| tests.cpp:615:2:615:7 | call to strcpy | main.cpp:6:27:6:30 | argv indirection | tests.cpp:615:17:615:22 | source indirection | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | argv indirection | a command-line argument |

‎cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ void test15()
407407
{
408408
if (ptr[5] == ' ') // GOOD
409409
{
410-
// ...
410+
break;
411411
}
412412
}
413413
}
@@ -608,6 +608,13 @@ int test23() {
608608
return sizeof(buffer) / sizeof(buffer[101]); // GOOD
609609
}
610610

611+
char* strcpy(char *, const char *);
612+
613+
void test24(char* source) {
614+
char buffer[100];
615+
strcpy(buffer, source); // BAD
616+
}
617+
611618
int tests_main(int argc, char *argv[])
612619
{
613620
long long arr17[19];
@@ -633,6 +640,7 @@ int tests_main(int argc, char *argv[])
633640
test21(argc == 0);
634641
test22(argc == 0, argv[0]);
635642
test23();
643+
test24(argv[0]);
636644

637645
return 0;
638646
}

‎go/ql/lib/change-notes/2023-10-31-add-gin-cors-framework.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added the [gin cors](https://github.com/gin-contrib/cors) library to the CorsMisconfiguration.ql query

‎go/ql/lib/go.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import semmle.go.frameworks.Email
4141
import semmle.go.frameworks.Encoding
4242
import semmle.go.frameworks.Fiber
4343
import semmle.go.frameworks.Gin
44+
import semmle.go.frameworks.GinCors
4445
import semmle.go.frameworks.Glog
4546
import semmle.go.frameworks.GoKit
4647
import semmle.go.frameworks.GoMicro

‎go/ql/lib/semmle/go/frameworks/GinCors.qll‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/**
2+
* Provides classes for modeling the `github.com/gin-contrib/cors` package.
3+
*/
4+
5+
import go
6+
7+
/**
8+
* Provides classes for modeling the `github.com/gin-contrib/cors` package.
9+
*/
10+
module GinCors {
11+
/** Gets the package name `github.com/gin-gonic/gin`. */
12+
string packagePath() { result = package("github.com/gin-contrib/cors", "") }
13+
14+
/**
15+
* A new function create a new gin Handler that passed to gin as middleware
16+
*/
17+
class New extends Function {
18+
New() { exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f) }
19+
}
20+
21+
/**
22+
* A write to the value of Access-Control-Allow-Credentials header
23+
*/
24+
class AllowCredentialsWrite extends DataFlow::ExprNode {
25+
DataFlow::Node base;
26+
27+
AllowCredentialsWrite() {
28+
exists(Field f, Write w |
29+
f.hasQualifiedName(packagePath(), "Config", "AllowCredentials") and
30+
w.writesField(base, f, this) and
31+
this.getType() instanceof BoolType
32+
)
33+
}
34+
35+
/**
36+
* Get config struct holding header values
37+
*/
38+
DataFlow::Node getBase() { result = base }
39+
40+
/**
41+
* Get config variable holding header values
42+
*/
43+
GinConfig getConfig() {
44+
exists(GinConfig gc |
45+
(
46+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
47+
base.asInstruction() or
48+
gc.getV().getAUse() = base
49+
) and
50+
result = gc
51+
)
52+
}
53+
}
54+
55+
/**
56+
* A write to the value of Access-Control-Allow-Origins header
57+
*/
58+
class AllowOriginsWrite extends DataFlow::ExprNode {
59+
DataFlow::Node base;
60+
61+
AllowOriginsWrite() {
62+
exists(Field f, Write w |
63+
f.hasQualifiedName(packagePath(), "Config", "AllowOrigins") and
64+
w.writesField(base, f, this) and
65+
this.asExpr() instanceof SliceLit
66+
)
67+
}
68+
69+
/**
70+
* Get config struct holding header values
71+
*/
72+
DataFlow::Node getBase() { result = base }
73+
74+
/**
75+
* Get config variable holding header values
76+
*/
77+
GinConfig getConfig() {
78+
exists(GinConfig gc |
79+
(
80+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
81+
base.asInstruction() or
82+
gc.getV().getAUse() = base
83+
) and
84+
result = gc
85+
)
86+
}
87+
}
88+
89+
/**
90+
* A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins
91+
*/
92+
class AllowAllOriginsWrite extends DataFlow::ExprNode {
93+
DataFlow::Node base;
94+
95+
AllowAllOriginsWrite() {
96+
exists(Field f, Write w |
97+
f.hasQualifiedName(packagePath(), "Config", "AllowAllOrigins") and
98+
w.writesField(base, f, this) and
99+
this.getType() instanceof BoolType
100+
)
101+
}
102+
103+
/**
104+
* Get config struct holding header values
105+
*/
106+
DataFlow::Node getBase() { result = base }
107+
108+
/**
109+
* Get config variable holding header values
110+
*/
111+
GinConfig getConfig() {
112+
exists(GinConfig gc |
113+
(
114+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
115+
base.asInstruction() or
116+
gc.getV().getAUse() = base
117+
) and
118+
result = gc
119+
)
120+
}
121+
}
122+
123+
/**
124+
* A variable of type Config that holds the headers to be set.
125+
*/
126+
class GinConfig extends Variable {
127+
SsaWithFields v;
128+
129+
GinConfig() {
130+
this = v.getBaseVariable().getSourceVariable() and
131+
exists(Type t | t.hasQualifiedName(packagePath(), "Config") | v.getType() = t)
132+
}
133+
134+
/**
135+
* Get variable declaration of GinConfig
136+
*/
137+
SsaWithFields getV() { result = v }
138+
}
139+
}

‎go/ql/src/experimental/CWE-287/ImproperLdapAuth.qhelp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
If an LDAP connection uses user-supplied data as password, anonymous bind could be caused using an empty password
6+
to result in a successful authentication.
7+
</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Don't use user-supplied data as password while establishing an LDAP connection.
12+
</p>
13+
</recommendation>
14+
15+
<example>
16+
<p>In the following examples, the code accepts a bind password via a HTTP request in variable <code>
17+
bindPassword</code>. The code builds a LDAP query whose authentication depends on user supplied data.</p>
18+
19+
20+
<sample src="examples/LdapAuthBad.go" />
21+
22+
<p>In the following examples, the code accepts a bind password via a HTTP request in variable <code>
23+
bindPassword</code>. The function ensures that the password provided is not empty before binding. </p>
24+
25+
<sample src="examples/LdapAuthGood.go" />
26+
</example>
27+
28+
<references>
29+
<li>MITRE: <a href="https://cwe.mitre.org/data/definitions/287.html">CWE-287: Improper Authentication</a>.</li>
30+
</references>
31+
</qhelp>

‎go/ql/src/experimental/CWE-287/ImproperLdapAuth.ql‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Improper LDAP Authentication
3+
* @description A user-controlled query carries no authentication
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @id go/improper-ldap-auth
7+
* @tags security
8+
* experimental
9+
* external/cwe/cwe-287
10+
*/
11+
12+
import go
13+
import ImproperLdapAuthCustomizations
14+
import ImproperLdapAuth::Flow::PathGraph
15+
16+
from ImproperLdapAuth::Flow::PathNode source, ImproperLdapAuth::Flow::PathNode sink
17+
where ImproperLdapAuth::Flow::flowPath(source, sink)
18+
select sink.getNode(), source, sink, "LDAP binding password depends on a $@.", source.getNode(),
19+
"user-provided value"

‎go/ql/src/experimental/CWE-287/ImproperLdapAuthCustomizations.qll‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import go
2+
import semmle.go.dataflow.barrierguardutil.RegexpCheck
3+
4+
module ImproperLdapAuth {
5+
/**
6+
* A sink that is vulnerable to improper LDAP Authentication vulnerabilities.
7+
*/
8+
abstract class LdapAuthSink extends DataFlow::Node { }
9+
10+
/**
11+
* A sanitizer function that prevents improper LDAP Authentication attacks.
12+
*/
13+
abstract class LdapSanitizer extends DataFlow::Node { }
14+
15+
/**
16+
* A vulnerable argument to `go-ldap` or `ldap`'s `bind` function (Only v2).
17+
*/
18+
private class GoLdapBindSink extends LdapAuthSink {
19+
GoLdapBindSink() {
20+
exists(Method meth |
21+
meth.hasQualifiedName("gopkg.in/ldap.v2", "Conn", "Bind") and
22+
this = meth.getACall().getArgument(1)
23+
)
24+
}
25+
}
26+
27+
/**
28+
* A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
29+
*
30+
* This is overapproximate: we do not attempt to reason about the correctness of the regexp.
31+
*/
32+
class RegexpCheckAsBarrierGuard extends RegexpCheckBarrier, LdapSanitizer { }
33+
34+
/**
35+
* An empty string.
36+
*/
37+
class EmptyString extends DataFlow::Node {
38+
EmptyString() { this.asExpr().getStringValue() = "" }
39+
}
40+
41+
private predicate equalityAsSanitizerGuard(DataFlow::Node g, Expr e, boolean outcome) {
42+
exists(DataFlow::Node nonConstNode, DataFlow::Node constNode, DataFlow::EqualityTestNode eq |
43+
g = eq and
44+
nonConstNode = eq.getAnOperand() and
45+
not nonConstNode.isConst() and
46+
constNode = eq.getAnOperand() and
47+
constNode.isConst() and
48+
e = nonConstNode.asExpr() and
49+
(
50+
// If `constNode` is not an empty string a comparison is considered a sanitizer
51+
not constNode instanceof EmptyString and outcome = eq.getPolarity()
52+
or
53+
// If `constNode` is an empty string a not comparison is considered a sanitizer
54+
constNode instanceof EmptyString and outcome = eq.getPolarity().booleanNot()
55+
)
56+
)
57+
}
58+
59+
/**
60+
* An equality check comparing a data-flow node against a constant string, considered as
61+
* a barrier guard for sanitizing untrusted user input.
62+
*/
63+
class EqualityAsSanitizerGuard extends LdapSanitizer {
64+
EqualityAsSanitizerGuard() {
65+
this = DataFlow::BarrierGuard<equalityAsSanitizerGuard/3>::getABarrierNode()
66+
}
67+
}
68+
69+
private module Config implements DataFlow::ConfigSig {
70+
predicate isSource(DataFlow::Node source) {
71+
source instanceof UntrustedFlowSource or source instanceof EmptyString
72+
}
73+
74+
predicate isSink(DataFlow::Node sink) { sink instanceof LdapAuthSink }
75+
76+
predicate isBarrier(DataFlow::Node node) { node instanceof LdapSanitizer }
77+
}
78+
79+
/**
80+
* Tracks taint flow for reasoning about improper ldap auth vulnerabilities
81+
* with sinks which are not sanitized by string comparisons.
82+
*/
83+
module Flow = TaintTracking::Global<Config>;
84+
}

‎go/ql/src/experimental/CWE-287/examples/LdapAuthBad.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"log"
6+
)
7+
8+
func bad() interface{} {
9+
bindPassword := req.URL.Query()["password"][0]
10+
11+
// Connect to the LDAP server
12+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
13+
if err != nil {
14+
log.Fatalf("Failed to connect to LDAP server: %v", err)
15+
}
16+
defer l.Close()
17+
18+
err = l.Bind("cn=admin,dc=example,dc=com", bindPassword)
19+
if err != nil {
20+
log.Fatalf("LDAP bind failed: %v", err)
21+
}
22+
}

‎go/ql/src/experimental/CWE-287/examples/LdapAuthGood.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"log"
6+
)
7+
8+
func good() interface{} {
9+
bindPassword := req.URL.Query()["password"][0]
10+
11+
// Connect to the LDAP server
12+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
13+
if err != nil {
14+
log.Fatalf("Failed to connect to LDAP server: %v", err)
15+
}
16+
defer l.Close()
17+
18+
if bindPassword != "" {
19+
err = l.Bind("cn=admin,dc=example,dc=com", bindPassword)
20+
if err != nil {
21+
log.Fatalf("LDAP bind failed: %v", err)
22+
}
23+
}
24+
}

0 commit comments

Comments
 (0)