From 41382dd7321738f6dd145bbe3c963cf7f5966fe5 Mon Sep 17 00:00:00 2001 From: calum Date: Thu, 23 Aug 2018 17:27:14 +0100 Subject: [PATCH 01/13] C#: A regression test for extractor crash when wrong expression type is used for a literal. --- csharp/ql/test/library-tests/regressions/Program.cs | 13 +++++++++++++ .../library-tests/regressions/TypeMentions.expected | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/csharp/ql/test/library-tests/regressions/Program.cs b/csharp/ql/test/library-tests/regressions/Program.cs index 88f57263a30..1f57d594c1f 100644 --- a/csharp/ql/test/library-tests/regressions/Program.cs +++ b/csharp/ql/test/library-tests/regressions/Program.cs @@ -81,3 +81,16 @@ class Designations return 0; } } + +class LiteralConversions +{ + struct Point + { + public int? x, y; + } + + void F() + { + new Point { x=1, y=2 }; + } +} diff --git a/csharp/ql/test/library-tests/regressions/TypeMentions.expected b/csharp/ql/test/library-tests/regressions/TypeMentions.expected index 76bb248e488..5f8f647903d 100644 --- a/csharp/ql/test/library-tests/regressions/TypeMentions.expected +++ b/csharp/ql/test/library-tests/regressions/TypeMentions.expected @@ -39,3 +39,9 @@ | Program.cs:69:5:69:8 | Boolean | | Program.cs:69:16:69:18 | Int32 | | Program.cs:75:5:75:7 | Int32 | +| Program.cs:89:16:89:18 | Int32 | +| Program.cs:89:16:89:18 | Int32 | +| Program.cs:89:16:89:19 | Nullable | +| Program.cs:89:16:89:19 | Nullable | +| Program.cs:92:5:92:8 | Void | +| Program.cs:94:13:94:17 | Point | From 71483c7025abae31082f6c270e1a49071197f380 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 4 Sep 2018 17:47:09 +0100 Subject: [PATCH 02/13] CPP: Remove some empty source files. --- cpp/ql/src/Architecture/InappropriateIntimacy.cpp | 0 cpp/ql/src/Documentation/UncommentedFunction.cpp | 1 - cpp/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.cpp | 1 - cpp/ql/src/Likely Bugs/Arithmetic/BitwiseSignCheck.cpp | 1 - 4 files changed, 3 deletions(-) delete mode 100644 cpp/ql/src/Architecture/InappropriateIntimacy.cpp delete mode 100644 cpp/ql/src/Documentation/UncommentedFunction.cpp delete mode 100644 cpp/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.cpp delete mode 100644 cpp/ql/src/Likely Bugs/Arithmetic/BitwiseSignCheck.cpp diff --git a/cpp/ql/src/Architecture/InappropriateIntimacy.cpp b/cpp/ql/src/Architecture/InappropriateIntimacy.cpp deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/cpp/ql/src/Documentation/UncommentedFunction.cpp b/cpp/ql/src/Documentation/UncommentedFunction.cpp deleted file mode 100644 index 8b137891791..00000000000 --- a/cpp/ql/src/Documentation/UncommentedFunction.cpp +++ /dev/null @@ -1 +0,0 @@ - diff --git a/cpp/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.cpp b/cpp/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.cpp deleted file mode 100644 index 8b137891791..00000000000 --- a/cpp/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.cpp +++ /dev/null @@ -1 +0,0 @@ - diff --git a/cpp/ql/src/Likely Bugs/Arithmetic/BitwiseSignCheck.cpp b/cpp/ql/src/Likely Bugs/Arithmetic/BitwiseSignCheck.cpp deleted file mode 100644 index 8b137891791..00000000000 --- a/cpp/ql/src/Likely Bugs/Arithmetic/BitwiseSignCheck.cpp +++ /dev/null @@ -1 +0,0 @@ - From c204ec3a8f1809ab1e5e68abd52481d760689b90 Mon Sep 17 00:00:00 2001 From: Ian Lynagh Date: Wed, 5 Sep 2018 12:28:57 +0100 Subject: [PATCH 03/13] C++: Enhance qualifiers/class-enum test --- .../qualifiers/class-enum/decls.expected | 23 +++++++++++++++++++ .../qualifiers/class-enum/decls.ql | 5 ++++ 2 files changed, 28 insertions(+) create mode 100644 cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected create mode 100644 cpp/ql/test/library-tests/qualifiers/class-enum/decls.ql diff --git a/cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected b/cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected new file mode 100644 index 00000000000..eb3476cc4e0 --- /dev/null +++ b/cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected @@ -0,0 +1,23 @@ +| file://:0:0:0:0 | __va_list_tag | __va_list_tag | +| file://:0:0:0:0 | fp_offset | __va_list_tag::fp_offset | +| file://:0:0:0:0 | gp_offset | __va_list_tag::gp_offset | +| file://:0:0:0:0 | operator= | __va_list_tag::operator= | +| file://:0:0:0:0 | operator= | __va_list_tag::operator= | +| file://:0:0:0:0 | overflow_arg_area | __va_list_tag::overflow_arg_area | +| file://:0:0:0:0 | reg_save_area | __va_list_tag::reg_save_area | +| test.cpp:2:7:2:7 | operator= | MyEnumClass::operator= | +| test.cpp:2:7:2:7 | operator= | MyEnumClass::operator= | +| test.cpp:2:7:2:17 | MyEnumClass | MyEnumClass | +| test.cpp:4:10:4:15 | MyEnum | MyEnumClass::MyEnum | +| test.cpp:5:9:5:9 | A | MyEnumClass::A | +| test.cpp:5:9:5:9 | A | MyEnumClass::MyEnum::A | +| test.cpp:6:9:6:9 | B | MyEnumClass::B | +| test.cpp:6:9:6:9 | B | MyEnumClass::MyEnum::B | +| test.cpp:10:34:10:34 | v | v | +| test.cpp:12:7:12:7 | MyClass2 | MyClass2::MyClass2 | +| test.cpp:12:7:12:7 | MyClass2 | MyClass2::MyClass2 | +| test.cpp:12:7:12:7 | operator= | MyClass2::operator= | +| test.cpp:12:7:12:7 | operator= | MyClass2::operator= | +| test.cpp:12:7:12:14 | MyClass2 | MyClass2 | +| test.cpp:14:12:14:19 | MyClass2 | MyClass2::MyClass2 | +| test.cpp:17:6:17:6 | f | f | diff --git a/cpp/ql/test/library-tests/qualifiers/class-enum/decls.ql b/cpp/ql/test/library-tests/qualifiers/class-enum/decls.ql new file mode 100644 index 00000000000..48db95ba5a2 --- /dev/null +++ b/cpp/ql/test/library-tests/qualifiers/class-enum/decls.ql @@ -0,0 +1,5 @@ +import cpp + +from Declaration d +select d, d.getQualifiedName() + From ca082be371baa608758805e1c8764097ff12fe6e Mon Sep 17 00:00:00 2001 From: Ian Lynagh Date: Wed, 5 Sep 2018 12:29:43 +0100 Subject: [PATCH 04/13] C++: Fix spurious extra qualified names for enum constants within a class --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 1 + cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 0576c6e2c53..2b42485eda6 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -56,6 +56,7 @@ abstract class Declaration extends Locatable, @declaration { // MemberFunction, MemberVariable, MemberType exists (Declaration m | m = this and + not m instanceof EnumConstant and result = m.getDeclaringType().getQualifiedName() + "::" + m.getName()) or exists (EnumConstant c diff --git a/cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected b/cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected index eb3476cc4e0..ddf489bd4df 100644 --- a/cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected +++ b/cpp/ql/test/library-tests/qualifiers/class-enum/decls.expected @@ -9,9 +9,7 @@ | test.cpp:2:7:2:7 | operator= | MyEnumClass::operator= | | test.cpp:2:7:2:17 | MyEnumClass | MyEnumClass | | test.cpp:4:10:4:15 | MyEnum | MyEnumClass::MyEnum | -| test.cpp:5:9:5:9 | A | MyEnumClass::A | | test.cpp:5:9:5:9 | A | MyEnumClass::MyEnum::A | -| test.cpp:6:9:6:9 | B | MyEnumClass::B | | test.cpp:6:9:6:9 | B | MyEnumClass::MyEnum::B | | test.cpp:10:34:10:34 | v | v | | test.cpp:12:7:12:7 | MyClass2 | MyClass2::MyClass2 | From 3d3b7b0254a17f4c5d1089f5ec7e5b3ec71f1211 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Thu, 6 Sep 2018 22:54:07 +0200 Subject: [PATCH 05/13] JS: fix typo in test case --- .../ql/test/query-tests/Security/CWE-730/RegExpInjection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js index 18dd843897b..5d1288f3a7e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js +++ b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js @@ -1,6 +1,6 @@ var express = require('express'); var app = express(); -var URI = reuires("urijs"); +var URI = require("urijs"); app.get('/findKey', function(req, res) { var key = req.param("key"), input = req.param("input"); From ecfc53668ff8dd10f0e0d34bb6f23b67c003958a Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Sep 2018 12:00:42 +0200 Subject: [PATCH 06/13] C++: IR: InstructionSanity::duplicateOperand perf The `InstructionSanity::duplicateOperand` predicate used `count` instead of `strictcount`. The 0-case of this `count` was as large as the Cartesian product of `Instruction` and `OperandTag`, which made `duplicateOperand` take forever to compute on large snapshots. --- .../code/cpp/ir/implementation/aliased_ssa/Instruction.qll | 2 +- .../src/semmle/code/cpp/ir/implementation/raw/Instruction.qll | 2 +- .../code/cpp/ir/implementation/unaliased_ssa/Instruction.qll | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll index 660c4ac3c9c..3d27026e3d0 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll @@ -63,7 +63,7 @@ module InstructionSanity { * Holds if instruction `instr` has multiple operands with tag `tag`. */ query predicate duplicateOperand(Instruction instr, OperandTag tag) { - count(instr.getOperand(tag)) > 1 and + strictcount(instr.getOperand(tag)) > 1 and not tag instanceof UnmodeledUseOperand } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll index 660c4ac3c9c..3d27026e3d0 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll @@ -63,7 +63,7 @@ module InstructionSanity { * Holds if instruction `instr` has multiple operands with tag `tag`. */ query predicate duplicateOperand(Instruction instr, OperandTag tag) { - count(instr.getOperand(tag)) > 1 and + strictcount(instr.getOperand(tag)) > 1 and not tag instanceof UnmodeledUseOperand } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll index 660c4ac3c9c..3d27026e3d0 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll @@ -63,7 +63,7 @@ module InstructionSanity { * Holds if instruction `instr` has multiple operands with tag `tag`. */ query predicate duplicateOperand(Instruction instr, OperandTag tag) { - count(instr.getOperand(tag)) > 1 and + strictcount(instr.getOperand(tag)) > 1 and not tag instanceof UnmodeledUseOperand } From d956bf90ade4d7b3b41d217f8b3d144639ff86b4 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Sep 2018 15:15:44 +0200 Subject: [PATCH 07/13] C++: Document the three predicates for array size --- cpp/ql/src/semmle/code/cpp/Type.qll | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Type.qll b/cpp/ql/src/semmle/code/cpp/Type.qll index e3df04abb5a..2d6176b7d7a 100644 --- a/cpp/ql/src/semmle/code/cpp/Type.qll +++ b/cpp/ql/src/semmle/code/cpp/Type.qll @@ -904,15 +904,26 @@ class ArrayType extends DerivedType { ArrayType() { derivedtypes(underlyingElement(this),_,4,_) } predicate hasArraySize() { arraysizes(underlyingElement(this),_,_,_) } + + /** + * Gets the number of elements in this array. Only has a result for arrays declared to be of a + * constant size. See `getByteSize` for getting the number of bytes. + */ int getArraySize() { arraysizes(underlyingElement(this),result,_,_) } + /** + * Gets the byte size of this array. Only has a result for arrays declared to be of a constant + * size. This predicate is a synonym for `getByteSize`. See `getArraySize` for getting the number + * of elements. + */ int getByteSize() { arraysizes(underlyingElement(this),_,result,_) } override int getAlignment() { arraysizes(underlyingElement(this), _, _, result) } /** - * Gets the size of this array (only valid for arrays declared to be of a constant - * size, will fail for all others). + * Gets the byte size of this array. Only has a result for arrays declared to be of a constant + * size. This predicate is a synonym for `getByteSize`. See `getArraySize` for getting the number + * of elements. */ override int getSize() { result = this.getByteSize() From 4304a4e1bc7f57aabb570a80ccbd2b598200d0ac Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Sep 2018 15:39:44 +0200 Subject: [PATCH 08/13] C++: Fix docs copy-paste error --- cpp/ql/src/semmle/code/cpp/Type.qll | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Type.qll b/cpp/ql/src/semmle/code/cpp/Type.qll index 2d6176b7d7a..060d7a27e97 100644 --- a/cpp/ql/src/semmle/code/cpp/Type.qll +++ b/cpp/ql/src/semmle/code/cpp/Type.qll @@ -913,8 +913,7 @@ class ArrayType extends DerivedType { /** * Gets the byte size of this array. Only has a result for arrays declared to be of a constant - * size. This predicate is a synonym for `getByteSize`. See `getArraySize` for getting the number - * of elements. + * size. See `getArraySize` for getting the number of elements. */ int getByteSize() { arraysizes(underlyingElement(this),_,result,_) } From 43c65e02ec0e1211a66dc9ae03b649b9d178f903 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Tue, 11 Sep 2018 13:21:36 +0200 Subject: [PATCH 09/13] JS: classify bundle files based on multiple license comments --- .../ql/src/semmle/javascript/frameworks/Bundling.qll | 7 +++++++ .../filters/ClassifyFiles/ClassifyFiles.expected | 1 + .../filters/ClassifyFiles/multiple-licenses-2.js | 10 ++++++++++ 3 files changed, 18 insertions(+) create mode 100644 javascript/ql/test/query-tests/filters/ClassifyFiles/multiple-licenses-2.js diff --git a/javascript/ql/src/semmle/javascript/frameworks/Bundling.qll b/javascript/ql/src/semmle/javascript/frameworks/Bundling.qll index 280f4013f6b..0adab311f6e 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Bundling.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Bundling.qll @@ -213,6 +213,7 @@ Comment getExclamationPointCommentInRun(ExclamationPointComment head) { * Holds if this is a bundle containing multiple licenses. */ predicate isMultiLicenseBundle(TopLevel tl) { + // case: comments preserved by minifiers count(ExclamationPointComment head | head.getTopLevel() = tl and exists(ExclamationPointComment licenseIndicator | @@ -220,6 +221,12 @@ predicate isMultiLicenseBundle(TopLevel tl) { licenseIndicator.getLine(_).regexpMatch("(?i).*\\b(copyright|license|\\d+\\.\\d+)\\b.*") ) ) > 1 + or + // case: ordinary block comments with "@license" lines + count(BlockComment head | + head.getTopLevel() = tl and + head.getLine(_).regexpMatch("(?i) *\\* @license .*") + ) > 1 } /** diff --git a/javascript/ql/test/query-tests/filters/ClassifyFiles/ClassifyFiles.expected b/javascript/ql/test/query-tests/filters/ClassifyFiles/ClassifyFiles.expected index 2efab05a9b1..5709a9ad1ff 100644 --- a/javascript/ql/test/query-tests/filters/ClassifyFiles/ClassifyFiles.expected +++ b/javascript/ql/test/query-tests/filters/ClassifyFiles/ClassifyFiles.expected @@ -11,6 +11,7 @@ | jsx.js:0:0:0:0 | jsx.js | generated | | multi-part-bundle.html:0:0:0:0 | multi-part-bundle.html | generated | | multi-part-bundle.js:0:0:0:0 | multi-part-bundle.js | generated | +| multiple-licenses-2.js:0:0:0:0 | multiple-licenses-2.js | generated | | multiple-licenses.js:0:0:0:0 | multiple-licenses.js | generated | | opal-test.js:0:0:0:0 | opal-test.js | generated | | peg-js.js:0:0:0:0 | peg-js.js | generated | diff --git a/javascript/ql/test/query-tests/filters/ClassifyFiles/multiple-licenses-2.js b/javascript/ql/test/query-tests/filters/ClassifyFiles/multiple-licenses-2.js new file mode 100644 index 00000000000..856c468f81a --- /dev/null +++ b/javascript/ql/test/query-tests/filters/ClassifyFiles/multiple-licenses-2.js @@ -0,0 +1,10 @@ +/* + * @copyright (c) ... + * @copyright (c) ... + * @license ... + */ + +/** + * @copyright ... + * @license ... + */ From 9fb5fbd995028334a815f47e68ed072effac9f2d Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 12 Sep 2018 09:29:56 +0200 Subject: [PATCH 10/13] C++: Restructure UnsafeUseOfStrcat for performance This query gets optimized badly, and it has started timing out when we run it on our own code base. Most of the evaluation time is spent in an RA predicate named `#select#cpe#1#f#antijoin_rhs#1`, which takes 1m36s a Wireshark snapshot. This restructuring of the code makes the problematic RA predicate go away. --- .../Memory Management/UnsafeUseOfStrcat.ql | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Memory Management/UnsafeUseOfStrcat.ql b/cpp/ql/src/Likely Bugs/Memory Management/UnsafeUseOfStrcat.ql index b3ab5828094..51302b710f2 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/UnsafeUseOfStrcat.ql +++ b/cpp/ql/src/Likely Bugs/Memory Management/UnsafeUseOfStrcat.ql @@ -29,11 +29,20 @@ predicate isEffectivelyConstAccess(VariableAccess a) ) } -from FunctionCall fc, VariableAccess src -where fc.getTarget().hasName("strcat") and - src = fc.getArgument(1) and - not src.getType() instanceof ArrayType and +class StrcatSource extends VariableAccess { + FunctionCall strcat; + + StrcatSource() { + strcat.getTarget().hasName("strcat") and + this = strcat.getArgument(1) + } + + FunctionCall getStrcatCall() { result = strcat } +} + +from StrcatSource src +where not src.getType() instanceof ArrayType and not exists(BufferSizeExpr bse | bse.getArg().(VariableAccess).getTarget() = src.getTarget()) and not isEffectivelyConstAccess(src) -select fc, "Always check the size of the source buffer when using strcat." +select src.getStrcatCall(), "Always check the size of the source buffer when using strcat." From ccbd8aaebc08456c5d23cfe0b666ee22ec3f2995 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 12 Sep 2018 10:13:57 +0200 Subject: [PATCH 11/13] Java: Improve alert message of IntMultToLong. --- java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql b/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql index acb99effb00..8027b9176a0 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql +++ b/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql @@ -52,4 +52,4 @@ where // not obviously small and ok not small(e) and e.getEnclosingCallable().fromSource() -select c, "$@ converted to "+ destType.getName() +" by use in " + ("a " + c.kind()).regexpReplaceAll("^a ([aeiou])", "an $1") + ".", e, sourceType.getName() + " multiplication" +select c, "Potential overflow in $@ before it is converted to "+ destType.getName() +" by use in " + ("a " + c.kind()).regexpReplaceAll("^a ([aeiou])", "an $1") + ".", e, sourceType.getName() + " multiplication" From 1bbc67b57c8f9136eade0694d8a86a7e4c0ae686 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 12 Sep 2018 10:14:41 +0200 Subject: [PATCH 12/13] Java: Autoformat query. --- java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql b/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql index 8027b9176a0..d77a98bcd06 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql +++ b/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql @@ -15,6 +15,7 @@ * external/cwe/cwe-197 * external/cwe/cwe-681 */ + import java import semmle.code.java.dataflow.RangeUtils import semmle.code.java.Conversions @@ -25,7 +26,8 @@ predicate small(MulExpr e) { lhs = e.getLeftOperand().getProperExpr().(ConstantIntegerExpr).getIntValue() and rhs = e.getRightOperand().getProperExpr().(ConstantIntegerExpr).getIntValue() and lhs * rhs = res and - t.getOrdPrimitiveType().getMinValue() <= res and res <= t.getOrdPrimitiveType().getMaxValue() + t.getOrdPrimitiveType().getMinValue() <= res and + res <= t.getOrdPrimitiveType().getMaxValue() ) } @@ -52,4 +54,7 @@ where // not obviously small and ok not small(e) and e.getEnclosingCallable().fromSource() -select c, "Potential overflow in $@ before it is converted to "+ destType.getName() +" by use in " + ("a " + c.kind()).regexpReplaceAll("^a ([aeiou])", "an $1") + ".", e, sourceType.getName() + " multiplication" +select c, + "Potential overflow in $@ before it is converted to " + destType.getName() + " by use in " + + ("a " + c.kind()).regexpReplaceAll("^a ([aeiou])", "an $1") + ".", e, + sourceType.getName() + " multiplication" From b9acdf573ab2fdea9ec1e9b216569c440e0f0615 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 13 Sep 2018 10:18:09 +0200 Subject: [PATCH 13/13] Java: Update qltest. --- .../security/CWE-190/semmle/tests/IntMultToLong.expected | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/IntMultToLong.expected b/java/ql/test/query-tests/security/CWE-190/semmle/tests/IntMultToLong.expected index e4a6143ee59..421575b07a6 100644 --- a/java/ql/test/query-tests/security/CWE-190/semmle/tests/IntMultToLong.expected +++ b/java/ql/test/query-tests/security/CWE-190/semmle/tests/IntMultToLong.expected @@ -1,4 +1,4 @@ -| Test.java:20:23:20:48 | ... * ... | $@ converted to long by use in an assignment context. | Test.java:20:23:20:48 | ... * ... | int multiplication | -| Test.java:27:23:27:52 | ... + ... | $@ converted to long by use in an assignment context. | Test.java:27:23:27:48 | ... * ... | int multiplication | -| Test.java:34:23:34:63 | ...?...:... | $@ converted to long by use in an assignment context. | Test.java:34:30:34:55 | ... * ... | int multiplication | -| Test.java:41:25:41:49 | ... * ... | $@ converted to double by use in an assignment context. | Test.java:41:25:41:49 | ... * ... | long multiplication | \ No newline at end of file +| Test.java:20:23:20:48 | ... * ... | Potential overflow in $@ before it is converted to long by use in an assignment context. | Test.java:20:23:20:48 | ... * ... | int multiplication | +| Test.java:27:23:27:52 | ... + ... | Potential overflow in $@ before it is converted to long by use in an assignment context. | Test.java:27:23:27:48 | ... * ... | int multiplication | +| Test.java:34:23:34:63 | ...?...:... | Potential overflow in $@ before it is converted to long by use in an assignment context. | Test.java:34:30:34:55 | ... * ... | int multiplication | +| Test.java:41:25:41:49 | ... * ... | Potential overflow in $@ before it is converted to double by use in an assignment context. | Test.java:41:25:41:49 | ... * ... | long multiplication |