From eeaa2bcc55453976dd9e5231a5bbe93df72992ad Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 29 Feb 2024 11:13:36 +0100 Subject: [PATCH 1/2] JS: Add test for class instance escaping into dependency --- .../library-tests/EndpointNaming/EndpointNaming.expected | 1 + .../ql/test/library-tests/EndpointNaming/pack1/main.js | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected b/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected index af5e8d62bb3a..87a41e7716be 100644 --- a/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected +++ b/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected @@ -1,4 +1,5 @@ testFailures +| pack1/main.js:19:6:19:10 | | Unexpected result: name=(pack1).InternalClass.prototype.m | ambiguousPreferredPredecessor | pack2/lib.js:1:1:3:1 | def moduleImport("pack2").getMember("exports").getMember("lib").getMember("LibClass").getInstance() | | pack2/lib.js:8:22:8:34 | def moduleImport("pack2").getMember("exports").getMember("lib").getMember("LibClass").getMember("foo") | diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack1/main.js b/javascript/ql/test/library-tests/EndpointNaming/pack1/main.js index cc550f0da15b..53269fb0602b 100644 --- a/javascript/ql/test/library-tests/EndpointNaming/pack1/main.js +++ b/javascript/ql/test/library-tests/EndpointNaming/pack1/main.js @@ -13,3 +13,9 @@ export function getEscapingInstance() { } // $ name=(pack1).getEscapingInstance export function publicFunction() {} // $ name=(pack1).publicFunction + +// Escapes into an upstream library, but is not exposed downstream +class InternalClass { + m() {} +} +require('foo').bar(new InternalClass()); From 6a0adff1dc97a2d98307c46844528f8a3c77d682 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 29 Feb 2024 11:10:52 +0100 Subject: [PATCH 2/2] JS: More precise detection of classes with escaping instances --- .../lib/semmle/javascript/endpoints/EndpointNaming.qll | 9 ++++++++- .../library-tests/EndpointNaming/EndpointNaming.expected | 1 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll b/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll index eb2fa714a394..fdb2b7ab9669 100644 --- a/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll +++ b/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll @@ -277,9 +277,16 @@ private predicate nameFromGlobal(DataFlow::Node node, string package, string nam (if node.getTopLevel().isExterns() then badness = -10 else badness = 10) } +/** Gets an API node whose value is exposed to client code. */ +private API::Node exposedNode() { + result = API::moduleExport(_) + or + result = exposedNode().getASuccessor() +} + /** Holds if an instance of `cls` can be exposed to client code. */ private predicate hasEscapingInstance(DataFlow::ClassNode cls) { - cls.getAnInstanceReference().flowsTo(any(API::Node n).asSink()) + cls.getAnInstanceReference().flowsTo(exposedNode().asSink()) } private predicate sourceNodeHasNameCandidate( diff --git a/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected b/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected index 87a41e7716be..af5e8d62bb3a 100644 --- a/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected +++ b/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected @@ -1,5 +1,4 @@ testFailures -| pack1/main.js:19:6:19:10 | | Unexpected result: name=(pack1).InternalClass.prototype.m | ambiguousPreferredPredecessor | pack2/lib.js:1:1:3:1 | def moduleImport("pack2").getMember("exports").getMember("lib").getMember("LibClass").getInstance() | | pack2/lib.js:8:22:8:34 | def moduleImport("pack2").getMember("exports").getMember("lib").getMember("LibClass").getMember("foo") |