Skip to content

Commit 06f86dd

Browse files
owen-mcowen-mc
committed
Convert squirrel sql-injection sinks to MaD (non-existent methods removed)
Various non-existent methods were modeled, and I couldn't find any evidence that they used to exist. They aren't in the stubs or tests. I have removed them.
1 parent f7d8c21 commit 06f86dd

File tree

3 files changed

+68
-45
lines changed

3 files changed

+68
-45
lines changed

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/go-all
4+
extensible: packageGrouping
5+
data:
6+
- ["squirrel", "github.com/Masterminds/squirrel"]
7+
- ["squirrel", "gopkg.in/Masterminds/squirrel"]
8+
- ["squirrel", "github.com/lann/squirrel"]
9+
- addsTo:
10+
pack: codeql/go-all
11+
extensible: sinkModel
12+
data:
13+
- ["group:squirrel", "", True, "Delete", "", "", "Argument[0]", "sql-injection", "manual"]
14+
- ["group:squirrel", "", True, "Expr", "", "", "Argument[0]", "sql-injection", "manual"]
15+
- ["group:squirrel", "", True, "Insert", "", "", "Argument[0]", "sql-injection", "manual"]
16+
- ["group:squirrel", "", True, "Select", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
17+
- ["group:squirrel", "", True, "Update", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
18+
19+
- ["group:squirrel", "DeleteBuilder", True, "From", "", "", "Argument[0]", "sql-injection", "manual"]
20+
- ["group:squirrel", "DeleteBuilder", True, "OrderBy", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
21+
- ["group:squirrel", "DeleteBuilder", True, "Prefix", "", "", "Argument[0]", "sql-injection", "manual"]
22+
- ["group:squirrel", "DeleteBuilder", True, "Suffix", "", "", "Argument[0]", "sql-injection", "manual"]
23+
- ["group:squirrel", "DeleteBuilder", True, "Where", "", "", "Argument[0]", "sql-injection", "manual"]
24+
25+
- ["group:squirrel", "InsertBuilder", True, "Columns", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
26+
- ["group:squirrel", "InsertBuilder", True, "Into", "", "", "Argument[0]", "sql-injection", "manual"]
27+
- ["group:squirrel", "InsertBuilder", True, "Options", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
28+
- ["group:squirrel", "InsertBuilder", True, "Prefix", "", "", "Argument[0]", "sql-injection", "manual"]
29+
- ["group:squirrel", "InsertBuilder", True, "Suffix", "", "", "Argument[0]", "sql-injection", "manual"]
30+
31+
- ["group:squirrel", "SelectBuilder", True, "CrossJoin", "", "", "Argument[0]", "sql-injection", "manual"]
32+
- ["group:squirrel", "SelectBuilder", True, "Column", "", "", "Argument[0]", "sql-injection", "manual"]
33+
- ["group:squirrel", "SelectBuilder", True, "Columns", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
34+
- ["group:squirrel", "SelectBuilder", True, "From", "", "", "Argument[0]", "sql-injection", "manual"]
35+
- ["group:squirrel", "SelectBuilder", True, "GroupBy", "", "", "Argument[0]", "sql-injection", "manual"]
36+
- ["group:squirrel", "SelectBuilder", True, "InnerJoin", "", "", "Argument[0]", "sql-injection", "manual"]
37+
- ["group:squirrel", "SelectBuilder", True, "LeftJoin", "", "", "Argument[0]", "sql-injection", "manual"]
38+
- ["group:squirrel", "SelectBuilder", True, "Options", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
39+
- ["group:squirrel", "SelectBuilder", True, "OrderBy", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
40+
- ["group:squirrel", "SelectBuilder", True, "Prefix", "", "", "Argument[0]", "sql-injection", "manual"]
41+
- ["group:squirrel", "SelectBuilder", True, "RightJoin", "", "", "Argument[0]", "sql-injection", "manual"]
42+
- ["group:squirrel", "SelectBuilder", True, "Suffix", "", "", "Argument[0]", "sql-injection", "manual"]
43+
- ["group:squirrel", "SelectBuilder", True, "Where", "", "", "Argument[0]", "sql-injection", "manual"]
44+
45+
- ["group:squirrel", "UpdateBuilder", True, "From", "", "", "Argument[0]", "sql-injection", "manual"]
46+
- ["group:squirrel", "UpdateBuilder", True, "OrderBy", "", "", "Argument[0]", "sql-injection", "manual"] # TODO: when sources can have access paths, use .ArrayElement
47+
- ["group:squirrel", "UpdateBuilder", True, "Prefix", "", "", "Argument[0]", "sql-injection", "manual"]
48+
- ["group:squirrel", "UpdateBuilder", True, "Set", "", "", "Argument[0]", "sql-injection", "manual"]
49+
- ["group:squirrel", "UpdateBuilder", True, "Suffix", "", "", "Argument[0]", "sql-injection", "manual"]
50+
- ["group:squirrel", "UpdateBuilder", True, "Table", "", "", "Argument[0]", "sql-injection", "manual"]
51+
- ["group:squirrel", "UpdateBuilder", True, "Where", "", "", "Argument[0]", "sql-injection", "manual"]

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

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -67,42 +67,14 @@ module SQL {
6767
*/
6868
abstract class Range extends DataFlow::Node { }
6969

70-
/**
71-
* An argument to an API of the squirrel library that is directly interpreted as SQL without
72-
* taking syntactic structure into account.
73-
*/
74-
private class SquirrelQueryString extends Range {
75-
SquirrelQueryString() {
76-
exists(Function fn |
77-
exists(string sq |
78-
sq =
79-
package([
80-
"github.com/Masterminds/squirrel", "gopkg.in/Masterminds/squirrel",
81-
"github.com/lann/squirrel"
82-
], "")
83-
|
84-
fn.hasQualifiedName(sq, ["Delete", "Expr", "Insert", "Select", "Update"])
85-
or
86-
exists(Method m, string builder | m = fn |
87-
builder = ["DeleteBuilder", "InsertBuilder", "SelectBuilder", "UpdateBuilder"] and
88-
m.hasQualifiedName(sq, builder,
89-
["Columns", "From", "Options", "OrderBy", "Prefix", "Suffix", "Where"])
90-
or
91-
builder = "InsertBuilder" and
92-
m.hasQualifiedName(sq, builder, ["Replace", "Into"])
93-
or
94-
builder = "SelectBuilder" and
95-
m.hasQualifiedName(sq, builder,
96-
["CrossJoin", "GroupBy", "InnerJoin", "LeftJoin", "RightJoin"])
97-
or
98-
builder = "UpdateBuilder" and
99-
m.hasQualifiedName(sq, builder, ["Set", "Table"])
100-
)
101-
) and
102-
this = fn.getACall().getArgument(0)
103-
|
104-
this.getType().getUnderlyingType() instanceof StringType or
105-
this.getType().getUnderlyingType().(SliceType).getElementType() instanceof StringType
70+
private class DefaultQueryString extends Range {
71+
DefaultQueryString() {
72+
exists(DataFlow::ArgumentNode arg | sinkNode(arg, "sql-injection") |
73+
not arg instanceof DataFlow::ImplicitVarargsSlice and
74+
this = arg
75+
or
76+
arg instanceof DataFlow::ImplicitVarargsSlice and
77+
this = arg.getCall().getAnImplicitVarargsArgument()
10678
)
10779
}
10880
}

‎go/ql/test/library-tests/semmle/go/frameworks/SQL/main.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,35 +46,35 @@ func squirrelTest(querypart string) {
4646
squirrel.Expr(querypart) // $ querystring=querypart
4747
deleteBuilder := squirrel.Delete(querypart) // $ querystring=querypart
4848
deleteBuilder.From(querypart) // $ querystring=querypart
49-
deleteBuilder.OrderBy(querypart) // $ querystring=[]type{args}
49+
deleteBuilder.OrderBy(querypart) // $ querystring=querypart
5050
deleteBuilder.Prefix(querypart) // $ querystring=querypart
5151
deleteBuilder.Suffix(querypart) // $ querystring=querypart
5252
deleteBuilder.Where(querypart) // $ querystring=querypart
5353

5454
insertBuilder := squirrel.Insert(querypart) // $ querystring=querypart
55-
insertBuilder.Columns(querypart) // $ querystring=[]type{args}
56-
insertBuilder.Options(querypart) // $ querystring=[]type{args}
55+
insertBuilder.Columns(querypart) // $ querystring=querypart
56+
insertBuilder.Options(querypart) // $ querystring=querypart
5757
insertBuilder.Prefix(querypart) // $ querystring=querypart
5858
insertBuilder.Suffix(querypart) // $ querystring=querypart
5959
insertBuilder.Into(querypart) // $ querystring=querypart
6060

61-
selectBuilder := squirrel.Select(querypart) // $ querystring=[]type{args}
62-
selectBuilder.Columns(querypart) // $ querystring=[]type{args}
61+
selectBuilder := squirrel.Select(querypart) // $ querystring=querypart
62+
selectBuilder.Columns(querypart) // $ querystring=querypart
6363
selectBuilder.From(querypart) // $ querystring=querypart
64-
selectBuilder.Options(querypart) // $ querystring=[]type{args}
65-
selectBuilder.OrderBy(querypart) // $ querystring=[]type{args}
64+
selectBuilder.Options(querypart) // $ querystring=querypart
65+
selectBuilder.OrderBy(querypart) // $ querystring=querypart
6666
selectBuilder.Prefix(querypart) // $ querystring=querypart
6767
selectBuilder.Suffix(querypart) // $ querystring=querypart
6868
selectBuilder.Where(querypart) // $ querystring=querypart
6969
selectBuilder.CrossJoin(querypart) // $ querystring=querypart
70-
selectBuilder.GroupBy(querypart) // $ querystring=[]type{args}
70+
selectBuilder.GroupBy(querypart) // $ querystring=querypart
7171
selectBuilder.InnerJoin(querypart) // $ querystring=querypart
7272
selectBuilder.LeftJoin(querypart) // $ querystring=querypart
7373
selectBuilder.RightJoin(querypart) // $ querystring=querypart
7474

7575
updateBuilder := squirrel.Update(querypart) // $ querystring=querypart
7676
updateBuilder.From(querypart) // $ querystring=querypart
77-
updateBuilder.OrderBy(querypart) // $ querystring=[]type{args}
77+
updateBuilder.OrderBy(querypart) // $ querystring=querypart
7878
updateBuilder.Prefix(querypart) // $ querystring=querypart
7979
updateBuilder.Suffix(querypart) // $ querystring=querypart
8080
updateBuilder.Where(querypart) // $ querystring=querypart

0 commit comments

Comments
 (0)