From 5c193b7dc9203367227be37c09817bebe9131efd Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 11 Nov 2019 16:28:21 -0800 Subject: [PATCH 1/3] Refactor for demo. --- .../FizzOverflow.ql | 29 +++++++++++-------- .../NarrowingConversions.ql | 20 +++++++++++++ 2 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql diff --git a/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql index 9039d2f..b0ec8b5 100644 --- a/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql +++ b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql @@ -22,26 +22,31 @@ class EndianConvert extends Function { } } +/** Holds if `i` is an endianness conversion. */ +predicate isEndiannessConversion(Instruction i) { + i.(CallInstruction).getCallTarget().(FunctionInstruction).getFunctionSymbol() + instanceof EndianConvert +} + +/** Holds if `i` is a narrowing conversion. */ +predicate isNarrowingConversion(ConvertInstruction i) { + i.getResultSize() < i.getUnary().getResultSize() +} + class Cfg extends TaintTracking::Configuration { Cfg() { this = "FizzOverflowIR" } - /** Holds if `source` is a call to `Endian::big()`. */ + /** + * Holds if `source` is an endianness conversion. + * (A telltale sign of network data.) + */ override predicate isSource(DataFlow::Node source) { - source - .asInstruction() - .(CallInstruction) - .getCallTarget() - .(FunctionInstruction) - .getFunctionSymbol() instanceof EndianConvert + isEndiannessConversion(source.asInstruction()) } /** Holds if `sink` is a narrowing conversion. */ override predicate isSink(DataFlow::Node sink) { - sink.asInstruction().getResultSize() < sink - .asInstruction() - .(ConvertInstruction) - .getUnary() - .getResultSize() + isNarrowingConversion(sink.asInstruction()) } } diff --git a/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql b/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql new file mode 100644 index 0000000..70c0a85 --- /dev/null +++ b/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql @@ -0,0 +1,20 @@ +/** + * @name Narrowing conversions + * @description Find all narrowing conversions from a larger integer type, + * such as uint32_t, to a smaller integer type, such as uint8_t. + */ + +import cpp +import semmle.code.cpp.ir.IR + +/** Holds if `i` is a narrowing conversion. */ +predicate isNarrowingConversion(ConvertInstruction i) { + i.getResultSize() < i.getUnary().getResultSize() +} + +from ConvertInstruction conv, Type inputType, Type outputType +where + isNarrowingConversion(conv) and + inputType = conv.getUnary().getResultType() and + outputType = conv.getResultType() +select conv, "Narrowing conversion from " + inputType + " to " + outputType + "." From 2db91d08913c1acddfba8aba11fa5a8462021e62 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Tue, 12 Nov 2019 16:13:24 -0800 Subject: [PATCH 2/3] More refactoring. --- .../cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql | 11 ++++++----- .../NarrowingConversions.ql | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql index b0ec8b5..4d0ee76 100644 --- a/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql +++ b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql @@ -22,8 +22,10 @@ class EndianConvert extends Function { } } -/** Holds if `i` is an endianness conversion. */ -predicate isEndiannessConversion(Instruction i) { +/** Holds if `i` is an endianness conversion. + * (A telltale sign of network data.) + */ +predicate isNetworkData(Instruction i) { i.(CallInstruction).getCallTarget().(FunctionInstruction).getFunctionSymbol() instanceof EndianConvert } @@ -37,11 +39,10 @@ class Cfg extends TaintTracking::Configuration { Cfg() { this = "FizzOverflowIR" } /** - * Holds if `source` is an endianness conversion. - * (A telltale sign of network data.) + * Holds if `source` is network data. */ override predicate isSource(DataFlow::Node source) { - isEndiannessConversion(source.asInstruction()) + isNetworkData(source.asInstruction()) } /** Holds if `sink` is a narrowing conversion. */ diff --git a/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql b/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql index 70c0a85..9fd37b7 100644 --- a/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql +++ b/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql @@ -2,6 +2,7 @@ * @name Narrowing conversions * @description Find all narrowing conversions from a larger integer type, * such as uint32_t, to a smaller integer type, such as uint8_t. + * @kind problem */ import cpp From e4ab7bb6c55e3900ab873f8c1d09a0e021ee8edd Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 18 Nov 2019 16:11:14 +0000 Subject: [PATCH 3/3] Work in progress. --- .../Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql | 15 ++++++--------- .../NarrowingConversions.ql | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) rename {ql_demos => CodeQL_Queries}/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql (88%) diff --git a/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql index 4d0ee76..bb46526 100644 --- a/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql +++ b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/FizzOverflow.ql @@ -22,12 +22,13 @@ class EndianConvert extends Function { } } -/** Holds if `i` is an endianness conversion. +/** + * Holds if `i` is an endianness conversion. * (A telltale sign of network data.) */ predicate isNetworkData(Instruction i) { - i.(CallInstruction).getCallTarget().(FunctionInstruction).getFunctionSymbol() - instanceof EndianConvert + i.(CallInstruction).getCallTarget().(FunctionInstruction).getFunctionSymbol() instanceof + EndianConvert } /** Holds if `i` is a narrowing conversion. */ @@ -41,14 +42,10 @@ class Cfg extends TaintTracking::Configuration { /** * Holds if `source` is network data. */ - override predicate isSource(DataFlow::Node source) { - isNetworkData(source.asInstruction()) - } + override predicate isSource(DataFlow::Node source) { isNetworkData(source.asInstruction()) } /** Holds if `sink` is a narrowing conversion. */ - override predicate isSink(DataFlow::Node sink) { - isNarrowingConversion(sink.asInstruction()) - } + override predicate isSink(DataFlow::Node sink) { isNarrowingConversion(sink.asInstruction()) } } from diff --git a/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql similarity index 88% rename from ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql rename to CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql index 9fd37b7..87ffe32 100644 --- a/ql_demos/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql +++ b/CodeQL_Queries/cpp/Facebook_Fizz_CVE-2019-3560/NarrowingConversions.ql @@ -1,7 +1,7 @@ /** * @name Narrowing conversions * @description Find all narrowing conversions from a larger integer type, - * such as uint32_t, to a smaller integer type, such as uint8_t. + * such as uint32_t, to a smaller integer type, such as uint8_t. * @kind problem */