Skip to content

Commit 25cd4d4

Browse files
committed
Model some squirrel methods in QL
We need to put a restriction on the type of the argument.
1 parent a0729fc commit 25cd4d4

File tree

2 files changed

+24
-3
lines changed

2 files changed

+24
-3
lines changed

go/ql/lib/ext/github.com.mastermind.squirrel.model.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ extensions:
2020
- ["group:squirrel", "DeleteBuilder", True, "OrderBy", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
2121
- ["group:squirrel", "DeleteBuilder", True, "Prefix", "", "", "Argument[0]", "sql-injection", "manual"]
2222
- ["group:squirrel", "DeleteBuilder", True, "Suffix", "", "", "Argument[0]", "sql-injection", "manual"]
23-
- ["group:squirrel", "DeleteBuilder", True, "Where", "", "", "Argument[0]", "sql-injection", "manual"]
23+
# DeleteBuilder.Where has to be modeled in QL to avoid FPs when a non-string argument is used
2424

2525
- ["group:squirrel", "InsertBuilder", True, "Columns", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
2626
- ["group:squirrel", "InsertBuilder", True, "Into", "", "", "Argument[0]", "sql-injection", "manual"]
@@ -40,12 +40,12 @@ extensions:
4040
- ["group:squirrel", "SelectBuilder", True, "Prefix", "", "", "Argument[0]", "sql-injection", "manual"]
4141
- ["group:squirrel", "SelectBuilder", True, "RightJoin", "", "", "Argument[0]", "sql-injection", "manual"]
4242
- ["group:squirrel", "SelectBuilder", True, "Suffix", "", "", "Argument[0]", "sql-injection", "manual"]
43-
- ["group:squirrel", "SelectBuilder", True, "Where", "", "", "Argument[0]", "sql-injection", "manual"]
43+
# SelectBuilder.Where has to be modeled in QL to avoid FPs when a non-string argument is used
4444

4545
- ["group:squirrel", "UpdateBuilder", True, "From", "", "", "Argument[0]", "sql-injection", "manual"]
4646
- ["group:squirrel", "UpdateBuilder", True, "OrderBy", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
4747
- ["group:squirrel", "UpdateBuilder", True, "Prefix", "", "", "Argument[0]", "sql-injection", "manual"]
4848
- ["group:squirrel", "UpdateBuilder", True, "Set", "", "", "Argument[0]", "sql-injection", "manual"]
4949
- ["group:squirrel", "UpdateBuilder", True, "Suffix", "", "", "Argument[0]", "sql-injection", "manual"]
5050
- ["group:squirrel", "UpdateBuilder", True, "Table", "", "", "Argument[0]", "sql-injection", "manual"]
51-
- ["group:squirrel", "UpdateBuilder", True, "Where", "", "", "Argument[0]", "sql-injection", "manual"]
51+
# UpdateBuilder.Where has to be modeled in QL to avoid FPs when a non-string argument is used

go/ql/lib/semmle/go/frameworks/SQL.qll

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,27 @@ module SQL {
7979
}
8080
}
8181

82+
/**
83+
* An argument to an API of the squirrel library that is directly interpreted as SQL without
84+
* taking syntactic structure into account.
85+
*/
86+
private class SquirrelQueryString extends Range {
87+
SquirrelQueryString() {
88+
exists(string sq, Method m, string builder |
89+
FlowExtensions::packageGrouping("squirrel", sq) and
90+
builder = ["DeleteBuilder", "SelectBuilder", "UpdateBuilder"]
91+
|
92+
m.hasQualifiedName(sq, builder, "Where") and
93+
this = m.getACall().getArgument(0)
94+
) and
95+
(
96+
this.getType().getUnderlyingType() instanceof StringType
97+
or
98+
this.getType().getUnderlyingType().(SliceType).getElementType() instanceof StringType
99+
)
100+
}
101+
}
102+
82103
/** A string that might identify package `go-pg/pg` or a specific version of it. */
83104
private string gopg() { result = package("github.com/go-pg/pg", "") }
84105

0 commit comments

Comments
 (0)