Skip to content

Commit a832730

Browse files
committed
Revert "Convert squirrel sql-injection sinks to MaD (non-existent methods removed)"
This reverts commit 06f86dd.
1 parent ab88b9b commit a832730

File tree

3 files changed

+45
-64
lines changed

3 files changed

+45
-64
lines changed

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

Lines changed: 0 additions & 51 deletions
This file was deleted.

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

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

70-
private class DefaultQueryString extends Range {
71-
DefaultQueryString() {
72-
exists(DataFlow::ArgumentNode arg | sinkNode(arg, "sql-injection") |
73-
this = arg.getACorrespondingSyntacticArgument()
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
74106
)
75107
}
76108
}

go/ql/test/library-tests/semmle/go/frameworks/SQL/squirrel.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,35 +10,35 @@ func squirrelTest(querypart string) {
1010
squirrel.Expr(querypart) // $ querystring=querypart
1111
deleteBuilder := squirrel.Delete(querypart) // $ querystring=querypart
1212
deleteBuilder.From(querypart) // $ querystring=querypart
13-
deleteBuilder.OrderBy(querypart) // $ querystring=querypart
13+
deleteBuilder.OrderBy(querypart) // $ querystring=[]type{args}
1414
deleteBuilder.Prefix(querypart) // $ querystring=querypart
1515
deleteBuilder.Suffix(querypart) // $ querystring=querypart
1616
deleteBuilder.Where(querypart) // $ querystring=querypart
1717

1818
insertBuilder := squirrel.Insert(querypart) // $ querystring=querypart
19-
insertBuilder.Columns(querypart) // $ querystring=querypart
20-
insertBuilder.Options(querypart) // $ querystring=querypart
19+
insertBuilder.Columns(querypart) // $ querystring=[]type{args}
20+
insertBuilder.Options(querypart) // $ querystring=[]type{args}
2121
insertBuilder.Prefix(querypart) // $ querystring=querypart
2222
insertBuilder.Suffix(querypart) // $ querystring=querypart
2323
insertBuilder.Into(querypart) // $ querystring=querypart
2424

25-
selectBuilder := squirrel.Select(querypart) // $ querystring=querypart
26-
selectBuilder.Columns(querypart) // $ querystring=querypart
25+
selectBuilder := squirrel.Select(querypart) // $ querystring=[]type{args}
26+
selectBuilder.Columns(querypart) // $ querystring=[]type{args}
2727
selectBuilder.From(querypart) // $ querystring=querypart
28-
selectBuilder.Options(querypart) // $ querystring=querypart
29-
selectBuilder.OrderBy(querypart) // $ querystring=querypart
28+
selectBuilder.Options(querypart) // $ querystring=[]type{args}
29+
selectBuilder.OrderBy(querypart) // $ querystring=[]type{args}
3030
selectBuilder.Prefix(querypart) // $ querystring=querypart
3131
selectBuilder.Suffix(querypart) // $ querystring=querypart
3232
selectBuilder.Where(querypart) // $ querystring=querypart
3333
selectBuilder.CrossJoin(querypart) // $ querystring=querypart
34-
selectBuilder.GroupBy(querypart) // $ querystring=querypart
34+
selectBuilder.GroupBy(querypart) // $ querystring=[]type{args}
3535
selectBuilder.InnerJoin(querypart) // $ querystring=querypart
3636
selectBuilder.LeftJoin(querypart) // $ querystring=querypart
3737
selectBuilder.RightJoin(querypart) // $ querystring=querypart
3838

3939
updateBuilder := squirrel.Update(querypart) // $ querystring=querypart
4040
updateBuilder.From(querypart) // $ querystring=querypart
41-
updateBuilder.OrderBy(querypart) // $ querystring=querypart
41+
updateBuilder.OrderBy(querypart) // $ querystring=[]type{args}
4242
updateBuilder.Prefix(querypart) // $ querystring=querypart
4343
updateBuilder.Suffix(querypart) // $ querystring=querypart
4444
updateBuilder.Where(querypart) // $ querystring=querypart

0 commit comments

Comments
 (0)