Skip to content

Commit a3f940d

Browse files
owen-mcowen-mc
committed
Fix TypeAssertionCheck to not block successor flow
1 parent b578c1d commit a3f940d

File tree

2 files changed

+31
-31
lines changed

2 files changed

+31
-31
lines changed

‎go/ql/lib/semmle/go/security/IncorrectIntegerConversionLib.qll

+8-4
Original file line numberDiff line numberDiff line change
@@ -290,13 +290,17 @@ private predicate integerTypeBound(IntegerType it, int bitSize, int architecture
290290
* the type assertion succeeded. If it is not checked then there will be a
291291
* run-time panic if the type assertion fails, so we can assume it succeeded.
292292
*/
293-
class TypeAssertionCheck extends DataFlow::ExprNode, FlowStateTransformer {
293+
class TypeAssertionCheck extends DataFlow::InstructionNode, FlowStateTransformer {
294294
IntegerType it;
295295

296296
TypeAssertionCheck() {
297-
exists(TypeAssertExpr tae |
298-
this = DataFlow::exprNode(tae.getExpr()) and
299-
it = tae.getTypeExpr().getType().getUnderlyingType()
297+
exists(IR::Instruction evalAssert, TypeAssertExpr assert |
298+
it = assert.getTypeExpr().getType().getUnderlyingType() and
299+
evalAssert = IR::evalExprInstruction(assert)
300+
|
301+
if exists(IR::extractTupleElement(evalAssert, _))
302+
then this.asInstruction() = IR::extractTupleElement(evalAssert, 0)
303+
else this.asInstruction() = evalAssert
300304
)
301305
}
302306

‎go/ql/test/query-tests/Security/CWE-681/IncorrectIntegerConversion.expected

+23-27
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@
143143
| IncorrectIntegerConversion.go:526:7:526:25 | type conversion | IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | IncorrectIntegerConversion.go:526:12:526:24 | type assertion | Incorrect conversion of a signed 64-bit integer from $@ to a lower bit size type int8 without an upper bound check. | IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | strconv.ParseInt |
144144
| IncorrectIntegerConversion.go:528:7:528:25 | type conversion | IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | IncorrectIntegerConversion.go:528:12:528:24 | type assertion | Incorrect conversion of a signed 64-bit integer from $@ to a lower bit size type int8 without an upper bound check. | IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | strconv.ParseInt |
145145
| IncorrectIntegerConversion.go:538:7:538:13 | type conversion | IncorrectIntegerConversion.go:533:2:533:38 | ... := ...[0] | IncorrectIntegerConversion.go:538:12:538:12 | v | Incorrect conversion of a signed 64-bit integer from $@ to a lower bit size type int8 without an upper bound check. | IncorrectIntegerConversion.go:533:2:533:38 | ... := ...[0] | strconv.ParseInt |
146+
| IncorrectIntegerConversion.go:540:7:540:14 | type conversion | IncorrectIntegerConversion.go:533:2:533:38 | ... := ...[0] | IncorrectIntegerConversion.go:540:13:540:13 | v | Incorrect conversion of a signed 64-bit integer from $@ to a lower bit size type int16 without an upper bound check. | IncorrectIntegerConversion.go:533:2:533:38 | ... := ...[0] | strconv.ParseInt |
146147
| Test64BitArchitectureBuildConstraints.go:26:7:26:17 | type conversion | Test64BitArchitectureBuildConstraints.go:22:3:22:50 | ... := ...[0] | Test64BitArchitectureBuildConstraints.go:26:11:26:16 | parsed | Incorrect conversion of an unsigned 64-bit integer from $@ to a lower bit size type int without an upper bound check. | Test64BitArchitectureBuildConstraints.go:22:3:22:50 | ... := ...[0] | strconv.ParseUint |
147148
| TestNoArchitectureBuildConstraints.go:16:7:16:19 | type conversion | TestNoArchitectureBuildConstraints.go:12:3:12:48 | ... := ...[0] | TestNoArchitectureBuildConstraints.go:16:13:16:18 | parsed | Incorrect conversion of an integer with architecture-dependent bit size from $@ to a lower bit size type int32 without an upper bound check. | TestNoArchitectureBuildConstraints.go:12:3:12:48 | ... := ...[0] | strconv.ParseInt |
148149
| TestNoArchitectureBuildConstraints.go:17:7:17:20 | type conversion | TestNoArchitectureBuildConstraints.go:12:3:12:48 | ... := ...[0] | TestNoArchitectureBuildConstraints.go:17:14:17:19 | parsed | Incorrect conversion of an integer with architecture-dependent bit size from $@ to a lower bit size type uint32 without an upper bound check. | TestNoArchitectureBuildConstraints.go:12:3:12:48 | ... := ...[0] | strconv.ParseInt |
@@ -367,32 +368,31 @@ edges
367368
| IncorrectIntegerConversion.go:440:2:440:60 | ... := ...[0] | IncorrectIntegerConversion.go:448:12:448:17 | parsed | provenance | |
368369
| IncorrectIntegerConversion.go:440:2:440:60 | ... := ...[0] | IncorrectIntegerConversion.go:449:13:449:18 | parsed | provenance | |
369370
| IncorrectIntegerConversion.go:493:2:493:38 | ... := ...[0] | IncorrectIntegerConversion.go:495:14:495:18 | input | provenance | |
370-
| IncorrectIntegerConversion.go:495:14:495:18 | input | IncorrectIntegerConversion.go:496:2:501:21 | definition of v | provenance | |
371+
| IncorrectIntegerConversion.go:495:14:495:18 | input | IncorrectIntegerConversion.go:500:13:500:13 | v | provenance | |
371372
| IncorrectIntegerConversion.go:495:14:495:18 | input | IncorrectIntegerConversion.go:502:2:504:13 | definition of v | provenance | Config |
372373
| IncorrectIntegerConversion.go:495:14:495:18 | input | IncorrectIntegerConversion.go:505:2:506:13 | definition of v | provenance | Config |
373-
| IncorrectIntegerConversion.go:495:14:495:18 | input | IncorrectIntegerConversion.go:507:2:508:21 | definition of v | provenance | |
374-
| IncorrectIntegerConversion.go:496:2:501:21 | definition of v | IncorrectIntegerConversion.go:500:13:500:13 | v | provenance | Config |
374+
| IncorrectIntegerConversion.go:495:14:495:18 | input | IncorrectIntegerConversion.go:508:12:508:12 | v | provenance | |
375375
| IncorrectIntegerConversion.go:500:13:500:13 | v | IncorrectIntegerConversion.go:501:12:501:12 | v | provenance | |
376-
| IncorrectIntegerConversion.go:501:12:501:12 | v | IncorrectIntegerConversion.go:501:12:501:20 | type assertion | provenance | |
376+
| IncorrectIntegerConversion.go:501:12:501:12 | v | IncorrectIntegerConversion.go:501:12:501:20 | type assertion | provenance | Config |
377377
| IncorrectIntegerConversion.go:502:2:504:13 | definition of v | IncorrectIntegerConversion.go:504:12:504:12 | v | provenance | |
378378
| IncorrectIntegerConversion.go:505:2:506:13 | definition of v | IncorrectIntegerConversion.go:506:12:506:12 | v | provenance | |
379-
| IncorrectIntegerConversion.go:507:2:508:21 | definition of v | IncorrectIntegerConversion.go:508:12:508:12 | v | provenance | Config |
380-
| IncorrectIntegerConversion.go:508:12:508:12 | v | IncorrectIntegerConversion.go:508:12:508:20 | type assertion | provenance | |
381-
| IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | IncorrectIntegerConversion.go:515:9:515:13 | input | provenance | |
382-
| IncorrectIntegerConversion.go:515:9:515:13 | input | IncorrectIntegerConversion.go:517:15:517:19 | input | provenance | |
383-
| IncorrectIntegerConversion.go:515:9:515:13 | input | IncorrectIntegerConversion.go:523:13:523:17 | input | provenance | Config |
384-
| IncorrectIntegerConversion.go:515:9:515:13 | input | IncorrectIntegerConversion.go:526:12:526:16 | input | provenance | Config |
385-
| IncorrectIntegerConversion.go:515:9:515:13 | input | IncorrectIntegerConversion.go:528:12:528:16 | input | provenance | Config |
386-
| IncorrectIntegerConversion.go:517:15:517:19 | input | IncorrectIntegerConversion.go:520:13:520:17 | input | provenance | Config |
379+
| IncorrectIntegerConversion.go:508:12:508:12 | v | IncorrectIntegerConversion.go:508:12:508:20 | type assertion | provenance | Config |
380+
| IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | IncorrectIntegerConversion.go:520:13:520:17 | input | provenance | |
381+
| IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | IncorrectIntegerConversion.go:523:13:523:17 | input | provenance | |
382+
| IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | IncorrectIntegerConversion.go:526:12:526:16 | input | provenance | |
383+
| IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | IncorrectIntegerConversion.go:528:12:528:16 | input | provenance | |
387384
| IncorrectIntegerConversion.go:520:13:520:17 | input | IncorrectIntegerConversion.go:521:12:521:16 | input | provenance | |
388-
| IncorrectIntegerConversion.go:521:12:521:16 | input | IncorrectIntegerConversion.go:521:12:521:24 | type assertion | provenance | |
385+
| IncorrectIntegerConversion.go:521:12:521:16 | input | IncorrectIntegerConversion.go:521:12:521:24 | type assertion | provenance | Config |
389386
| IncorrectIntegerConversion.go:523:13:523:17 | input | IncorrectIntegerConversion.go:524:12:524:16 | input | provenance | |
390-
| IncorrectIntegerConversion.go:524:12:524:16 | input | IncorrectIntegerConversion.go:524:12:524:24 | type assertion | provenance | |
391-
| IncorrectIntegerConversion.go:526:12:526:16 | input | IncorrectIntegerConversion.go:526:12:526:24 | type assertion | provenance | |
392-
| IncorrectIntegerConversion.go:528:12:528:16 | input | IncorrectIntegerConversion.go:528:12:528:24 | type assertion | provenance | |
393-
| IncorrectIntegerConversion.go:533:2:533:38 | ... := ...[0] | IncorrectIntegerConversion.go:534:6:534:10 | definition of input | provenance | |
394-
| IncorrectIntegerConversion.go:534:6:534:10 | definition of input | IncorrectIntegerConversion.go:535:14:535:18 | input | provenance | Config |
395-
| IncorrectIntegerConversion.go:535:14:535:18 | input | IncorrectIntegerConversion.go:538:12:538:12 | v | provenance | |
387+
| IncorrectIntegerConversion.go:524:12:524:16 | input | IncorrectIntegerConversion.go:524:12:524:24 | type assertion | provenance | Config |
388+
| IncorrectIntegerConversion.go:526:12:526:16 | input | IncorrectIntegerConversion.go:526:12:526:24 | type assertion | provenance | Config |
389+
| IncorrectIntegerConversion.go:528:12:528:16 | input | IncorrectIntegerConversion.go:528:12:528:24 | type assertion | provenance | Config |
390+
| IncorrectIntegerConversion.go:533:2:533:38 | ... := ...[0] | IncorrectIntegerConversion.go:535:14:535:18 | input | provenance | |
391+
| IncorrectIntegerConversion.go:535:5:535:26 | ... := ...[0] | IncorrectIntegerConversion.go:538:12:538:12 | v | provenance | |
392+
| IncorrectIntegerConversion.go:535:14:535:18 | input | IncorrectIntegerConversion.go:535:5:535:26 | ... := ...[0] | provenance | Config |
393+
| IncorrectIntegerConversion.go:535:14:535:18 | input | IncorrectIntegerConversion.go:539:21:539:25 | input | provenance | |
394+
| IncorrectIntegerConversion.go:539:12:539:33 | ... := ...[0] | IncorrectIntegerConversion.go:540:13:540:13 | v | provenance | |
395+
| IncorrectIntegerConversion.go:539:21:539:25 | input | IncorrectIntegerConversion.go:539:12:539:33 | ... := ...[0] | provenance | Config |
396396
| Test64BitArchitectureBuildConstraints.go:22:3:22:50 | ... := ...[0] | Test64BitArchitectureBuildConstraints.go:26:11:26:16 | parsed | provenance | |
397397
| TestNoArchitectureBuildConstraints.go:12:3:12:48 | ... := ...[0] | TestNoArchitectureBuildConstraints.go:16:13:16:18 | parsed | provenance | |
398398
| TestNoArchitectureBuildConstraints.go:12:3:12:48 | ... := ...[0] | TestNoArchitectureBuildConstraints.go:17:14:17:19 | parsed | provenance | |
@@ -635,20 +635,16 @@ nodes
635635
| IncorrectIntegerConversion.go:449:13:449:18 | parsed | semmle.label | parsed |
636636
| IncorrectIntegerConversion.go:493:2:493:38 | ... := ...[0] | semmle.label | ... := ...[0] |
637637
| IncorrectIntegerConversion.go:495:14:495:18 | input | semmle.label | input |
638-
| IncorrectIntegerConversion.go:496:2:501:21 | definition of v | semmle.label | definition of v |
639638
| IncorrectIntegerConversion.go:500:13:500:13 | v | semmle.label | v |
640639
| IncorrectIntegerConversion.go:501:12:501:12 | v | semmle.label | v |
641640
| IncorrectIntegerConversion.go:501:12:501:20 | type assertion | semmle.label | type assertion |
642641
| IncorrectIntegerConversion.go:502:2:504:13 | definition of v | semmle.label | definition of v |
643642
| IncorrectIntegerConversion.go:504:12:504:12 | v | semmle.label | v |
644643
| IncorrectIntegerConversion.go:505:2:506:13 | definition of v | semmle.label | definition of v |
645644
| IncorrectIntegerConversion.go:506:12:506:12 | v | semmle.label | v |
646-
| IncorrectIntegerConversion.go:507:2:508:21 | definition of v | semmle.label | definition of v |
647645
| IncorrectIntegerConversion.go:508:12:508:12 | v | semmle.label | v |
648646
| IncorrectIntegerConversion.go:508:12:508:20 | type assertion | semmle.label | type assertion |
649647
| IncorrectIntegerConversion.go:513:2:513:38 | ... := ...[0] | semmle.label | ... := ...[0] |
650-
| IncorrectIntegerConversion.go:515:9:515:13 | input | semmle.label | input |
651-
| IncorrectIntegerConversion.go:517:15:517:19 | input | semmle.label | input |
652648
| IncorrectIntegerConversion.go:520:13:520:17 | input | semmle.label | input |
653649
| IncorrectIntegerConversion.go:521:12:521:16 | input | semmle.label | input |
654650
| IncorrectIntegerConversion.go:521:12:521:24 | type assertion | semmle.label | type assertion |
@@ -660,9 +656,12 @@ nodes
660656
| IncorrectIntegerConversion.go:528:12:528:16 | input | semmle.label | input |
661657
| IncorrectIntegerConversion.go:528:12:528:24 | type assertion | semmle.label | type assertion |
662658
| IncorrectIntegerConversion.go:533:2:533:38 | ... := ...[0] | semmle.label | ... := ...[0] |
663-
| IncorrectIntegerConversion.go:534:6:534:10 | definition of input | semmle.label | definition of input |
659+
| IncorrectIntegerConversion.go:535:5:535:26 | ... := ...[0] | semmle.label | ... := ...[0] |
664660
| IncorrectIntegerConversion.go:535:14:535:18 | input | semmle.label | input |
665661
| IncorrectIntegerConversion.go:538:12:538:12 | v | semmle.label | v |
662+
| IncorrectIntegerConversion.go:539:12:539:33 | ... := ...[0] | semmle.label | ... := ...[0] |
663+
| IncorrectIntegerConversion.go:539:21:539:25 | input | semmle.label | input |
664+
| IncorrectIntegerConversion.go:540:13:540:13 | v | semmle.label | v |
666665
| Test64BitArchitectureBuildConstraints.go:22:3:22:50 | ... := ...[0] | semmle.label | ... := ...[0] |
667666
| Test64BitArchitectureBuildConstraints.go:26:11:26:16 | parsed | semmle.label | parsed |
668667
| TestNoArchitectureBuildConstraints.go:12:3:12:48 | ... := ...[0] | semmle.label | ... := ...[0] |
@@ -674,6 +673,3 @@ nodes
674673
| TestOldBuildConstraints.go:23:3:23:50 | ... := ...[0] | semmle.label | ... := ...[0] |
675674
| TestOldBuildConstraints.go:27:11:27:16 | parsed | semmle.label | parsed |
676675
subpaths
677-
testFailures
678-
| IncorrectIntegerConversion.go:540:16:540:30 | comment | Missing result: Alert |
679-
| IncorrectIntegerConversion.go:540:16:540:30 | comment | Missing result: Sink |

0 commit comments

Comments
 (0)