Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Commit d300ec6

Browse files
author
Max Schaefer
committed
Refine Method.implements so that interface methods only implement themselves.
Without this restriction, the two `m`s in the following example are considered to implement each other, even though they aren't logically related: ```go type I interface { m() } type J interface { m() } type K struct { I J } ``` Previously, interface methods would sometimes implement themselves and sometimes not (see changes to test output for examples).
1 parent 87c1bca commit d300ec6

11 files changed

Lines changed: 67 additions & 13 deletions

File tree

ql/src/semmle/go/Scopes.qll

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,9 @@ class Method extends Function {
406406
result = this.getReceiverType().getPackage()
407407
}
408408

409+
/** Holds if this method is declared in an interface. */
410+
predicate isInterfaceMethod() { getReceiverType().getUnderlyingType() instanceof InterfaceType }
411+
409412
/** Gets the receiver variable of this method. */
410413
Variable getReceiver() { result = receiver }
411414

@@ -464,8 +467,14 @@ class Method extends Function {
464467
* Holds if this method implements the method `m`, that is, if `m` is a method
465468
* on an interface, and this is a method with the same name on a type that
466469
* implements that interface.
470+
*
471+
* Note that all methods implement themselves, and interface methods _only_
472+
* implement themselves.
467473
*/
468474
predicate implements(Method m) {
475+
this = m
476+
or
477+
not isInterfaceMethod() and
469478
exists(Type t |
470479
this = t.getMethod(m.getName()) and
471480
t.implements(m.getReceiverType().getUnderlyingType())

ql/test/library-tests/semmle/go/Scopes/DeclaredEntity.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,5 @@
2323
| types.go:33:16:33:20 | meth1 | types.go:33:16:33:20 | meth1 |
2424
| types.go:33:22:33:22 | a | types.go:33:22:33:22 | a |
2525
| types.go:37:16:37:20 | meth2 | types.go:37:16:37:20 | meth2 |
26+
| types.go:41:6:41:27 | iHaveARedeclaredMethod | types.go:41:6:41:27 | iHaveARedeclaredMethod |
27+
| types.go:43:2:43:5 | meth | types.go:43:2:43:5 | meth |

ql/test/library-tests/semmle/go/Scopes/EntityReference.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
| file://:0:0:0:0 | int | <library> | types.go:27:25:27:27 | int |
1414
| file://:0:0:0:0 | int | <library> | types.go:33:24:33:26 | int |
1515
| file://:0:0:0:0 | int | <library> | types.go:37:24:37:26 | int |
16+
| file://:0:0:0:0 | int | <library> | types.go:43:9:43:11 | int |
1617
| main.go:5:6:5:6 | t | main.go@5:6:5:6 | main.go:5:6:5:6 | t |
1718
| main.go:5:6:5:6 | t | main.go@5:6:5:6 | main.go:13:13:13:13 | t |
1819
| main.go:5:6:5:6 | t | main.go@5:6:5:6 | main.go:17:29:17:29 | t |
@@ -42,6 +43,7 @@
4243
| main.go:23:16:23:19 | bump | main.go@23:16:23:19 | main.go:23:16:23:19 | bump |
4344
| types.go:3:6:3:17 | iHaveAMethod | types.go@3:6:3:17 | main.go:17:12:17:23 | iHaveAMethod |
4445
| types.go:3:6:3:17 | iHaveAMethod | types.go@3:6:3:17 | types.go:3:6:3:17 | iHaveAMethod |
46+
| types.go:3:6:3:17 | iHaveAMethod | types.go@3:6:3:17 | types.go:42:2:42:13 | iHaveAMethod |
4547
| types.go:4:2:4:5 | meth | types.go@4:2:4:5 | main.go:18:2:18:7 | selection of meth |
4648
| types.go:4:2:4:5 | meth | types.go@4:2:4:5 | main.go:18:4:18:7 | meth |
4749
| types.go:4:2:4:5 | meth | types.go@4:2:4:5 | types.go:4:2:4:5 | meth |
@@ -65,3 +67,5 @@
6567
| types.go:33:22:33:22 | a | types.go@33:22:33:22 | types.go:33:22:33:22 | a |
6668
| types.go:33:22:33:22 | a | types.go@33:22:33:22 | types.go:34:9:34:9 | a |
6769
| types.go:37:16:37:20 | meth2 | types.go@37:16:37:20 | types.go:37:16:37:20 | meth2 |
70+
| types.go:41:6:41:27 | iHaveARedeclaredMethod | types.go@41:6:41:27 | types.go:41:6:41:27 | iHaveARedeclaredMethod |
71+
| types.go:43:2:43:5 | meth | types.go@43:2:43:5 | types.go:43:2:43:5 | meth |

ql/test/library-tests/semmle/go/Scopes/EntityType.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,5 @@
2323
| types.go:33:16:33:20 | meth1 | func(int) bool |
2424
| types.go:33:22:33:22 | a | int |
2525
| types.go:37:16:37:20 | meth2 | func() int |
26+
| types.go:41:6:41:27 | iHaveARedeclaredMethod | iHaveARedeclaredMethod |
27+
| types.go:43:2:43:5 | meth | func() int |
Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
| iHaveAMethod | meth | iHaveAMethod | meth |
2+
| iHaveARedeclaredMethod | meth | iHaveARedeclaredMethod | meth |
23
| meth1Iface | meth1 | meth1Iface | meth1 |
3-
| meth1Iface | meth1 | twoMethods | meth1 |
4+
| notImpl | meth1 | notImpl | meth1 |
5+
| notImpl | meth2 | notImpl | meth2 |
6+
| pointer type | bump | pointer type | bump |
47
| pointer type | meth | iHaveAMethod | meth |
8+
| pointer type | meth | iHaveARedeclaredMethod | meth |
9+
| pointer type | meth | pointer type | meth |
510
| pointer type | meth1 | meth1Iface | meth1 |
11+
| pointer type | meth1 | pointer type | meth1 |
612
| pointer type | meth1 | twoMethods | meth1 |
13+
| starImpl | meth2 | starImpl | meth2 |
714
| starImpl | meth2 | twoMethods | meth2 |
8-
| twoMethods | meth1 | meth1Iface | meth1 |
915
| twoMethods | meth1 | twoMethods | meth1 |
1016
| twoMethods | meth2 | twoMethods | meth2 |
Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
| iHaveAMethod | meth | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | iHaveAMethod | meth |
2+
| iHaveARedeclaredMethod | meth | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | iHaveARedeclaredMethod | meth |
23
| meth1Iface | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | meth1Iface | meth1 |
3-
| meth1Iface | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethods | meth1 |
44
| meth1Iface | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethodsEmbedded | meth1 |
5+
| notImpl | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | notImpl | meth1 |
6+
| notImpl | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | notImpl | meth2 |
7+
| pointer type | bump | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | t | bump |
58
| pointer type | meth | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | iHaveAMethod | meth |
9+
| pointer type | meth | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | iHaveARedeclaredMethod | meth |
10+
| pointer type | meth | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | t | meth |
611
| pointer type | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | meth1Iface | meth1 |
12+
| pointer type | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | starImpl | meth1 |
713
| pointer type | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethods | meth1 |
814
| pointer type | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethodsEmbedded | meth1 |
15+
| starImpl | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | starImpl | meth2 |
916
| starImpl | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethods | meth2 |
1017
| starImpl | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethodsEmbedded | meth2 |
11-
| twoMethods | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | meth1Iface | meth1 |
1218
| twoMethods | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethods | meth1 |
13-
| twoMethods | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethodsEmbedded | meth1 |
1419
| twoMethods | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethods | meth2 |
1520
| twoMethods | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes | twoMethodsEmbedded | meth2 |

ql/test/library-tests/semmle/go/Scopes/Methods.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@
1010
| types.go:27:17:27:21 | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes.starImpl.meth2 | file://:0:0:0:0 | | starImpl |
1111
| types.go:33:16:33:20 | meth1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes.notImpl.meth1 | file://:0:0:0:0 | | notImpl |
1212
| types.go:37:16:37:20 | meth2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes.notImpl.meth2 | file://:0:0:0:0 | | notImpl |
13+
| types.go:43:2:43:5 | meth | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Scopes.iHaveARedeclaredMethod.meth | file://:0:0:0:0 | | iHaveARedeclaredMethod |

ql/test/library-tests/semmle/go/Scopes/TypeImplements.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@
22
| * starImpl | twoMethods |
33
| * starImpl | twoMethodsEmbedded |
44
| * t | iHaveAMethod |
5+
| * t | iHaveARedeclaredMethod |
56
| iHaveAMethod | iHaveAMethod |
7+
| iHaveAMethod | iHaveARedeclaredMethod |
8+
| iHaveARedeclaredMethod | iHaveAMethod |
9+
| iHaveARedeclaredMethod | iHaveARedeclaredMethod |
610
| interface { meth1 func() bool } | meth1Iface |
711
| interface { meth1 func() bool; meth2 func() int } | meth1Iface |
812
| interface { meth1 func() bool; meth2 func() int } | twoMethods |
913
| interface { meth1 func() bool; meth2 func() int } | twoMethodsEmbedded |
1014
| interface { meth func() int } | iHaveAMethod |
15+
| interface { meth func() int } | iHaveARedeclaredMethod |
1116
| meth1Iface | meth1Iface |
1217
| twoMethods | meth1Iface |
1318
| twoMethods | twoMethods |

ql/test/library-tests/semmle/go/Scopes/types.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,8 @@ func (notImpl) meth1(a int) bool {
3737
func (notImpl) meth2() int {
3838
return -42
3939
}
40+
41+
type iHaveARedeclaredMethod interface {
42+
iHaveAMethod
43+
meth() int
44+
}

ql/test/query-tests/Security/CWE-079/ReflectedXss.expected

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,22 @@ edges
22
| ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:44:14:51 | username |
33
| contenttype.go:11:11:11:16 | selection of Form : Values | contenttype.go:17:11:17:22 | type conversion |
44
| contenttype.go:49:11:49:16 | selection of Form : Values | contenttype.go:53:34:53:37 | data |
5-
| tst.go:13:15:13:20 | selection of Form : Values | tst.go:17:12:17:39 | type conversion |
6-
| tst.go:47:14:47:19 | selection of Form : Values | tst.go:52:12:52:26 | type conversion |
5+
| tst.go:14:15:14:20 | selection of Form : Values | tst.go:18:12:18:39 | type conversion |
6+
| tst.go:48:14:48:19 | selection of Form : Values | tst.go:53:12:53:26 | type conversion |
77
nodes
88
| ReflectedXss.go:11:15:11:20 | selection of Form : Values | semmle.label | selection of Form : Values |
99
| ReflectedXss.go:14:44:14:51 | username | semmle.label | username |
1010
| contenttype.go:11:11:11:16 | selection of Form : Values | semmle.label | selection of Form : Values |
1111
| contenttype.go:17:11:17:22 | type conversion | semmle.label | type conversion |
1212
| contenttype.go:49:11:49:16 | selection of Form : Values | semmle.label | selection of Form : Values |
1313
| contenttype.go:53:34:53:37 | data | semmle.label | data |
14-
| tst.go:13:15:13:20 | selection of Form : Values | semmle.label | selection of Form : Values |
15-
| tst.go:17:12:17:39 | type conversion | semmle.label | type conversion |
16-
| tst.go:47:14:47:19 | selection of Form : Values | semmle.label | selection of Form : Values |
17-
| tst.go:52:12:52:26 | type conversion | semmle.label | type conversion |
14+
| tst.go:14:15:14:20 | selection of Form : Values | semmle.label | selection of Form : Values |
15+
| tst.go:18:12:18:39 | type conversion | semmle.label | type conversion |
16+
| tst.go:48:14:48:19 | selection of Form : Values | semmle.label | selection of Form : Values |
17+
| tst.go:53:12:53:26 | type conversion | semmle.label | type conversion |
1818
#select
1919
| ReflectedXss.go:14:44:14:51 | username | ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:44:14:51 | username | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:11:15:11:20 | selection of Form | user-provided value |
2020
| contenttype.go:17:11:17:22 | type conversion | contenttype.go:11:11:11:16 | selection of Form : Values | contenttype.go:17:11:17:22 | type conversion | Cross-site scripting vulnerability due to $@. | contenttype.go:11:11:11:16 | selection of Form | user-provided value |
2121
| contenttype.go:53:34:53:37 | data | contenttype.go:49:11:49:16 | selection of Form : Values | contenttype.go:53:34:53:37 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:49:11:49:16 | selection of Form | user-provided value |
22-
| tst.go:17:12:17:39 | type conversion | tst.go:13:15:13:20 | selection of Form : Values | tst.go:17:12:17:39 | type conversion | Cross-site scripting vulnerability due to $@. | tst.go:13:15:13:20 | selection of Form | user-provided value |
23-
| tst.go:52:12:52:26 | type conversion | tst.go:47:14:47:19 | selection of Form : Values | tst.go:52:12:52:26 | type conversion | Cross-site scripting vulnerability due to $@. | tst.go:47:14:47:19 | selection of Form | user-provided value |
22+
| tst.go:18:12:18:39 | type conversion | tst.go:14:15:14:20 | selection of Form : Values | tst.go:18:12:18:39 | type conversion | Cross-site scripting vulnerability due to $@. | tst.go:14:15:14:20 | selection of Form | user-provided value |
23+
| tst.go:53:12:53:26 | type conversion | tst.go:48:14:48:19 | selection of Form : Values | tst.go:53:12:53:26 | type conversion | Cross-site scripting vulnerability due to $@. | tst.go:48:14:48:19 | selection of Form | user-provided value |

0 commit comments

Comments
 (0)