Skip to content

Commit 611f98b

Browse files
committed
Make type assertions transform the flow state
1 parent 5446603 commit 611f98b

File tree

2 files changed

+41
-8
lines changed

2 files changed

+41
-8
lines changed

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,39 @@ class UpperBoundCheck extends FlowStateTransformer {
397397
}
398398
}
399399

400+
private predicate integerTypeBound(IntegerType it, int bitSize, int architectureBitSize) {
401+
bitSize = validBitSize() and
402+
architectureBitSize = [32, 64] and
403+
exists(int offset | if it instanceof SignedIntegerType then offset = 1 else offset = 0 |
404+
if it instanceof IntType or it instanceof UintType
405+
then bitSize >= architectureBitSize - offset
406+
else bitSize >= it.getSize() - offset
407+
)
408+
}
409+
410+
/**
411+
* An expression which a type assertion guarantees will have a particular
412+
* integer type.
413+
*
414+
* If this is a checked type expression then this value will only be used if
415+
* the type assertion succeeded. If it is not checked then there will be a
416+
* run-time panic if the type assertion fails, so we can assume it succeeded.
417+
*/
418+
class TypeAssertionCheck extends DataFlow::ExprNode, FlowStateTransformer {
419+
IntegerType it;
420+
421+
TypeAssertionCheck() {
422+
exists(TypeAssertExpr tae |
423+
this = DataFlow::exprNode(tae.getExpr()) and
424+
it = tae.getTypeExpr().getType()
425+
)
426+
}
427+
428+
override predicate barrierFor(int bitSize, int architectureBitSize) {
429+
integerTypeBound(it, bitSize, architectureBitSize)
430+
}
431+
}
432+
400433
/**
401434
* Holds if `source` is the result of a call to `strconv.Atoi`,
402435
* `strconv.ParseInt`, or `strconv.ParseUint`, `bitSize` is the `bitSize`

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,8 @@ func typeSwitch1(s string) {
507507
if _, ok := input.(string); ok {
508508
return
509509
}
510-
_ = int16(v.(int16)) // $ SPURIOUS: hasValueFlow="type assertion"
511-
_ = int8(v.(int16)) // $ hasValueFlow="type assertion"
510+
_ = int16(v.(int16))
511+
_ = int8(v.(int16)) // $ hasValueFlow="type assertion"
512512
case int32:
513513
_ = int32(v) // $ SPURIOUS: hasValueFlow="v"
514514
_ = int8(v) // $ hasValueFlow="v"
@@ -527,11 +527,11 @@ func typeSwitch2(s string) {
527527
if _, ok := input.(string); ok {
528528
return
529529
}
530-
_ = int16(input.(int16)) // $ SPURIOUS: hasValueFlow="type assertion"
531-
_ = int8(input.(int16)) // $ hasValueFlow="type assertion"
530+
_ = int16(input.(int16))
531+
_ = int8(input.(int16)) // $ hasValueFlow="type assertion"
532532
case int32:
533-
_ = int32(input.(int32)) // $ SPURIOUS: hasValueFlow="type assertion"
534-
_ = int8(input.(int32)) // $ hasValueFlow="type assertion"
533+
_ = int32(input.(int32))
534+
_ = int8(input.(int32)) // $ hasValueFlow="type assertion"
535535
case int64:
536536
_ = int8(input.(int64)) // $ hasValueFlow="type assertion"
537537
default:
@@ -544,8 +544,8 @@ func checkedTypeAssertion(s string) {
544544
var input any = i64
545545
if v, ok := input.(int16); ok {
546546
// Need to account for the fact that within this case clause, v is an int16
547-
_ = int16(v) // $ SPURIOUS: hasValueFlow="v"
548-
_ = int8(v) // $ hasValueFlow="v"
547+
_ = int16(v)
548+
_ = int8(v) // $ hasValueFlow="v"
549549
} else if v, ok := input.(int32); ok {
550550
_ = int16(v) // $ hasValueFlow="v"
551551
}

0 commit comments

Comments
 (0)