From 6e681f829b2b6eab4fd75df3255d8769d6dcd92d Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 12 Mar 2020 07:38:30 -0700 Subject: [PATCH 1/3] MistypedExponentiation: Add a heuristic to reduce FPs --- ql/src/InconsistentCode/MistypedExponentiation.ql | 4 ++++ .../InconsistentCode/MistypedExponentiation/main.go | 2 ++ 2 files changed, 6 insertions(+) diff --git a/ql/src/InconsistentCode/MistypedExponentiation.ql b/ql/src/InconsistentCode/MistypedExponentiation.ql index 3fbb7b34d15..3fefabc076a 100644 --- a/ql/src/InconsistentCode/MistypedExponentiation.ql +++ b/ql/src/InconsistentCode/MistypedExponentiation.ql @@ -32,6 +32,10 @@ where exists(Ident id | id = xe.getRightOperand() | id.getName().regexpMatch("(?i)_*((exp(onent)?)|pow(er)?)") ) + ) and + // exclude the right hand side of assignments to variables that have "mask" in their name + not exists(Assignment assign | assign.getRhs() = xe.getParent*() | + assign.getLhs().getAChild*().(Ident).getName().regexpMatch(".*(^m|M)ask($|\\p{Lu}).*") ) select xe, "This expression uses the bitwise exclusive-or operator when exponentiation was likely meant." diff --git a/ql/test/query-tests/InconsistentCode/MistypedExponentiation/main.go b/ql/test/query-tests/InconsistentCode/MistypedExponentiation/main.go index a749a8090f7..c9eca38b021 100644 --- a/ql/test/query-tests/InconsistentCode/MistypedExponentiation/main.go +++ b/ql/test/query-tests/InconsistentCode/MistypedExponentiation/main.go @@ -20,6 +20,8 @@ func main() { fmt.Println(253 ^ expectingResponse) // OK fmt.Println(2 ^ power) // Not OK + mask := (((1 << 10) - 1) ^ 7) // OK + // This is not ok, but isn't detected because the multiplication binds tighter // than the xor operator and so the query doesn't see a constant on the left // hand side of ^. From 630d0cef891a2692f3cc0d4aab474daee044be48 Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 12 Mar 2020 09:13:15 -0700 Subject: [PATCH 2/3] Address review comments --- ql/src/InconsistentCode/MistypedExponentiation.ql | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ql/src/InconsistentCode/MistypedExponentiation.ql b/ql/src/InconsistentCode/MistypedExponentiation.ql index 3fefabc076a..ab9a5608369 100644 --- a/ql/src/InconsistentCode/MistypedExponentiation.ql +++ b/ql/src/InconsistentCode/MistypedExponentiation.ql @@ -34,8 +34,12 @@ where ) ) and // exclude the right hand side of assignments to variables that have "mask" in their name - not exists(Assignment assign | assign.getRhs() = xe.getParent*() | - assign.getLhs().getAChild*().(Ident).getName().regexpMatch(".*(^m|M)ask($|\\p{Lu}).*") + not exists(Assignment assign, Ident id | assign.getRhs() = xe.getParent*() | + id.getName().regexpMatch("(?i).*mask.*") and + ( + assign.getLhs() = id or + assign.getLhs().(SelectorExpr).getSelector() = id + ) ) select xe, "This expression uses the bitwise exclusive-or operator when exponentiation was likely meant." From ea5e6a324d6d2b0b277746049d2052f1389d80da Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Fri, 13 Mar 2020 03:10:55 -0700 Subject: [PATCH 3/3] Add change note --- change-notes/1.24/analysis-go.md | 1 + 1 file changed, 1 insertion(+) diff --git a/change-notes/1.24/analysis-go.md b/change-notes/1.24/analysis-go.md index d8eea2c7d06..f19aec0a778 100644 --- a/change-notes/1.24/analysis-go.md +++ b/change-notes/1.24/analysis-go.md @@ -27,3 +27,4 @@ The CodeQL library for Go now contains a folder of simple "cookbook" queries tha | Incomplete regular expression for hostnames (`go/incomplete-hostname-regexp`) | More results | The query now flags unescaped dots before the TLD in a hostname regex. | | Reflected cross-site scripting (`go/reflected-xss`) | Fewer results | Untrusted input flowing into an HTTP header definition or into an `fmt.Fprintf` call with a constant prefix is no longer flagged, since it is in both cases often harmless. | | Useless assignment to field (`go/useless-assignment-to-field`) | Fewer false positives | The query now conservatively handles fields promoted through embedded pointer types. | +| Bitwise exclusive-or used like exponentiation (`go/mistyped-exponentiation`) | Fewer false positives | The query now identifies when the value of an xor is assigned to a mask object, and excludes such results. |