From 9421476beadfab08dcec30560dbf8bd24de38949 Mon Sep 17 00:00:00 2001 From: Slavomir Date: Wed, 1 Jul 2020 10:21:16 +0300 Subject: [PATCH] Add IndexExpr logic and example --- .../experimental/Unsafe/WrongUsageOfUnsafe.ql | 23 +++++++++----- .../experimental/Unsafe/WrongUsageOfUnsafe.go | 31 ++++++++++++++++--- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/ql/src/experimental/Unsafe/WrongUsageOfUnsafe.ql b/ql/src/experimental/Unsafe/WrongUsageOfUnsafe.ql index d3060bb0ba1..f183dd36b3a 100644 --- a/ql/src/experimental/Unsafe/WrongUsageOfUnsafe.ql +++ b/ql/src/experimental/Unsafe/WrongUsageOfUnsafe.ql @@ -54,22 +54,31 @@ predicate castShortArrayToLongerArray( ) { exists( UnsafeTypeCastingConf cfg, ConversionExpr castBig, ConversionToUnsafePointer castLittle, - ArrayType arrTo, ArrayType arrFrom + ArrayType arrTo, ArrayType arrFrom, int arrFromAvailableSize | cfg.hasFlowPath(source, sink) and cfg.isSource(source.getNode(), castLittle) and cfg.isSink(sink.getNode(), castBig) and arrTo = getBaseType(castBig.getTypeExpr().getType()) and ( - arrFrom = getBaseType(castLittle.getOperand().getType()) + arrFrom = getBaseType(castLittle.getOperand().getType()) and + arrFromAvailableSize = arrFrom.getLength() and + message = + "Dangerous array type casting to [" + arrTo.getLength() + "]" + arrTo.getElementType() + + " from [" + arrFrom.getLength() + "]" + arrFrom.getElementType() or - arrFrom = castLittle.getOperand().getChildExpr(0).(IndexExpr).getBase().getType() + exists(IndexExpr indexExpr | + indexExpr = castLittle.getOperand().getChildExpr(0) and + arrFrom = indexExpr.getBase().getType() and + arrFromAvailableSize = arrFrom.getLength() - indexExpr.getIndex().getIntValue() and + message = + "Dangerous array type casting to [" + arrTo.getLength() + "]" + arrTo.getElementType() + + " from an index [" + arrFrom.getLength() + "]" + arrFrom.getElementType() + "[" + + indexExpr.getIndex().getIntValue() + "]" + ) ) and arrTo.getLength() > 0 and //TODO - arrTo.getLength() > arrFrom.getLength() and - message = - "Dangerous array type casting to [" + arrTo.getLength() + "]" + arrTo.getElementType() + - " from [" + arrFrom.getLength() + "]" + arrFrom.getElementType() + arrTo.getLength() > arrFromAvailableSize ) } diff --git a/ql/test/experimental/Unsafe/WrongUsageOfUnsafe.go b/ql/test/experimental/Unsafe/WrongUsageOfUnsafe.go index 93252b5ba83..cf7226a3748 100644 --- a/ql/test/experimental/Unsafe/WrongUsageOfUnsafe.go +++ b/ql/test/experimental/Unsafe/WrongUsageOfUnsafe.go @@ -14,9 +14,9 @@ func good0() { // Read before secret without overflowing to secret: // NOTE: unsafe.Pointer(&harmless) == unsafe.Pointer(&harmless[0]) - var leaking = (*[8]byte)(unsafe.Pointer(&harmless)) // OK + var nonLeaking = (*[8]byte)(unsafe.Pointer(&harmless)) // OK - fmt.Println(string((*leaking)[:])) + fmt.Println(string((*nonLeaking)[:])) // Avoid optimization: if secret[0] == 123 { @@ -31,7 +31,28 @@ func good1() { // Read before secret without overflowing to secret: // NOTE: unsafe.Pointer(&harmless) == unsafe.Pointer(&harmless[0]) - var leaking = (*[8]byte)(unsafe.Pointer(&harmless[0])) // OK + var nonLeaking = (*[8]byte)(unsafe.Pointer(&harmless[0])) // OK + + fmt.Println(string((*nonLeaking)[:])) + + // Avoid optimization: + if secret[0] == 123 { + fmt.Println("hello world") + } +} +func badIndex() { + // A harmless piece of data: + harmless := [8]byte{'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A'} + // Something secret: + secret := [9]byte{'s', 'e', 'n', 's', 'i', 't', 'i', 'v', 'e'} + + // Read before secret, overflowing into secret. + // NOTE: unsafe.Pointer(&harmless) != unsafe.Pointer(&harmless[2]) + // The new variable `leaking` will contain data starting from + // the address of the 3rd element of the `harmless` array, + // and continue for 8 bytes, crossing the boundaries of + // `harmless` into the memory occupied by `secret`. + var leaking = (*[8]byte)(unsafe.Pointer(&harmless[2])) // BAD fmt.Println(string((*leaking)[:])) @@ -47,9 +68,9 @@ func good2() { secret := [9]byte{'s', 'e', 'n', 's', 'i', 't', 'i', 'v', 'e'} // Read before secret: - var leaking = (*int)(unsafe.Pointer(&harmless)) // TODO: is this really OK? + var nonLeaking = (*int)(unsafe.Pointer(&harmless)) // TODO: is this really OK? - fmt.Println(*leaking) + fmt.Println(*nonLeaking) // Avoid optimization: if secret[0] == 123 {