Skip to content

LGTM deprecation: a few more references missed in earlier PRs #11503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ Select expressions that cast a value to a type parameter:
where assertion.getTypeAnnotation() = param.getLocalTypeName().getAnAccess()
select assertion, "Cast to type parameter."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/1505979606441/>`__.

Classes and interfaces
~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -179,7 +177,7 @@ Ambient nodes are mostly ignored by control flow and data flow analysis. The out
Static type information
-----------------------

Static type information and global name binding is available for projects with "full" TypeScript extraction enabled. This option is enabled by default for projects on LGTM.com and when you create databases with the :ref:`CodeQL CLI <codeql-cli>`.
Static type information and global name binding is available for projects with "full" TypeScript extraction enabled. This option is enabled by default when you create databases with the :ref:`CodeQL CLI <codeql-cli>`.

Basic usage
~~~~~~~~~~~
Expand Down Expand Up @@ -403,8 +401,6 @@ It is best to use `TypeName <https://codeql.github.com/codeql-standard-libraries
and not access.hasTypeArguments()
select access, "Type arguments are omitted"

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/1505985316500/>`__.

Find imported names that are used as both a type and a value:

.. code-block:: ql
Expand All @@ -416,8 +412,6 @@ Find imported names that are used as both a type and a value:
and exists (VarAccess access | access.getVariable().getADeclaration() = spec.getLocal())
select spec, "Used as both variable and type"

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/1505975787348/>`__.

Namespace names
~~~~~~~~~~~~~~~

Expand Down
12 changes: 6 additions & 6 deletions docs/codeql/codeql-language-guides/navigating-the-call-graph.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ We can use the ``Callable`` class to write a query that finds methods that are n
where not exists(Callable caller | caller.polyCalls(callee))
select callee

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/8376915232270534450/>`__. This simple query typically returns a large number of results.
This simple query typically returns a large number of results.

.. pull-quote::

Expand All @@ -99,7 +99,7 @@ Running this query on a typical Java project results in lots of hits in the Java
callee.getCompilationUnit().fromSource()
select callee, "Not called."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/8711624074465690976/>`__. This change reduces the number of results returned for most projects.
This change reduces the number of results returned for most codebases.

We might also notice several unused methods with the somewhat strange name ``<clinit>``: these are class initializers; while they are not explicitly called anywhere in the code, they are called implicitly whenever the surrounding class is loaded. Hence it makes sense to exclude them from our query. While we are at it, we can also exclude finalizers, which are similarly invoked implicitly:

Expand All @@ -113,7 +113,7 @@ We might also notice several unused methods with the somewhat strange name ``<cl
not callee.hasName("<clinit>") and not callee.hasName("finalize")
select callee, "Not called."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/925473733866047471/>`__. This also reduces the number of results returned by most projects.
This also reduces the number of results returned by most codebases.

We may also want to exclude public methods from our query, since they may be external API entry points:

Expand All @@ -128,7 +128,7 @@ We may also want to exclude public methods from our query, since they may be ext
not callee.isPublic()
select callee, "Not called."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/6284320987237954610/>`__. This should have a more noticeable effect on the number of results returned.
This should have a more noticeable effect on the number of results returned.

A further special case is non-public default constructors: in the singleton pattern, for example, a class is provided with private empty default constructor to prevent it from being instantiated. Since the very purpose of such constructors is their not being called, they should not be flagged up:

Expand All @@ -144,7 +144,7 @@ A further special case is non-public default constructors: in the singleton patt
not callee.(Constructor).getNumberOfParameters() = 0
select callee, "Not called."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/2625028545869146918/>`__. This change has a large effect on the results for some projects but little effect on the results for others. Use of this pattern varies widely between different projects.
This change has a large effect on the results for some projects but little effect on the results for others. Use of this pattern varies widely between different projects.

Finally, on many Java projects there are methods that are invoked indirectly by reflection. So, while there are no calls invoking these methods, they are, in fact, used. It is in general very hard to identify such methods. A very common special case, however, is JUnit test methods, which are reflectively invoked by a test runner. The CodeQL library for Java has support for recognizing test classes of JUnit and other testing frameworks, which we can employ to filter out methods defined in such classes:

Expand All @@ -161,7 +161,7 @@ Finally, on many Java projects there are methods that are invoked indirectly by
not callee.getDeclaringType() instanceof TestClass
select callee, "Not called."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/2055862421970264112/>`__. This should give a further reduction in the number of results returned.
This should give a further reduction in the number of results returned.

Further reading
---------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,9 @@ step by step in the UI:
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Property access on JSON value originating $@.", source, "here"

`Here <https://lgtm.com/query/5347702611074820306>`_ is a run of this query on the `plexus-interop
<https://lgtm.com/projects/g/finos-plexus/plexus-interop/>`_ project on LGTM.com. Many of the 19
results are false positives since we currently do not model many ways in which a value can be
checked for nullness. In particular, after a property reference ``x.p`` we implicitly know that
We ran this query on the https://github.com/finos/plexus-interop repository. Many of the
results were false positives since the query does not currently model many ways in which we can check
a value for nullness. In particular, after a property reference ``x.p`` we implicitly know that
``x`` cannot be null anymore, since otherwise the reference would have thrown an exception.
Modeling this would allow us to get rid of most of the false positives, but is beyond the scope of
this tutorial.
Expand Down Expand Up @@ -391,10 +390,10 @@ Some of our standard security queries use flow labels. You can look at their imp
to get a feeling for how to use flow labels in practice.

In particular, both of the examples mentioned in the section on limitations of basic data flow above
are from standard security queries that use flow labels. The `Prototype pollution
<https://lgtm.com/rules/1508857356317>`_ query uses two flow labels to distinguish completely
are from standard security queries that use flow labels. The `Prototype-polluting merge call
<https://codeql.github.com/codeql-query-help/javascript/js-prototype-pollution/>`_ query uses two flow labels to distinguish completely
tainted objects from partially tainted objects. The `Uncontrolled data used in path expression
<https://lgtm.com/rules/1971530250>`_ query uses four flow labels to track whether a user-controlled
<https://codeql.github.com/codeql-query-help/javascript/js-path-injection/>`_ query uses four flow labels to track whether a user-controlled
string may be an absolute path and whether it may contain ``..`` components.

Further reading
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,23 @@ Here's see an example of what this can handle now:

Tracking in the whole model
---------------------------
We applied this pattern to ``firebaseDatabase()`` in the previous section, and it
can just as easily apply to the other predicates.
For reference, here's our simple Firebase model with type tracking on every predicate:
We applied this pattern to ``firebaseDatabase()`` in the previous section, and we can
apply the model just as easily to other predicates.
This example query uses the model to find `set` calls.
It's been modified slightly to handle a bit more of the API, which is beyond the scope of this tutorial.

.. code-block:: ql

import javascript
import DataFlow

SourceNode firebase(TypeTracker t) {
t.start() and
result = globalVarRef("firebase")
(
result = globalVarRef("firebase")
or
result = moduleImport("firebase/app")
)
or
exists(TypeTracker t2 |
result = firebase(t2).track(t2, t)
Expand Down Expand Up @@ -277,8 +285,7 @@ For reference, here's our simple Firebase model with type tracking on every pred
result = firebaseRef().getAMethodCall("set")
}

`Here <https://lgtm.com/query/1053770500827789481>`__ is a run of an example query using the model to find `set` calls on one of the Firebase sample projects.
It's been modified slightly to handle a bit more of the API, which is beyond the scope of this tutorial.
select firebaseSetterCall()

Tracking associated data
------------------------
Expand Down Expand Up @@ -392,7 +399,98 @@ Based on that we can track the ``snapshot`` value and find the ``val()`` call it

With this addition, ``firebaseDatabaseRead("forecast")`` finds the call to ``snapshot.val()`` that contains the value of the forecast.

`Here <https://lgtm.com/query/8761360814276109092>`__ is a run of an example query using the model to find `val` calls.
.. code-block:: ql

import javascript
import DataFlow

SourceNode firebase(TypeTracker t) {
t.start() and
(
result = globalVarRef("firebase")
or
result = moduleImport("firebase/app")
)
or
exists(TypeTracker t2 |
result = firebase(t2).track(t2, t)
)
}

SourceNode firebase() {
result = firebase(TypeTracker::end())
}

SourceNode firebaseDatabase(TypeTracker t) {
t.start() and
result = firebase().getAMethodCall("database")
or
exists(TypeTracker t2 |
result = firebaseDatabase(t2).track(t2, t)
)
}

SourceNode firebaseDatabase() {
result = firebaseDatabase(TypeTracker::end())
}

SourceNode firebaseRef(Node name, TypeTracker t) {
t.start() and
exists(CallNode call |
call = firebaseDatabase().getAMethodCall("ref") and
name = call.getArgument(0) and
result = call
)
or
exists(TypeTracker t2 |
result = firebaseRef(name, t2).track(t2, t)
)
}

SourceNode firebaseRef(Node name) {
result = firebaseRef(name, TypeTracker::end())
}

MethodCallNode firebaseSetterCall(Node name) {
result = firebaseRef(name).getAMethodCall("set")
}

SourceNode firebaseSnapshotCallback(Node refName, TypeBackTracker t) {
t.start() and
(
result = firebaseRef(refName).getAMethodCall("once").getArgument(1).getALocalSource()
or
result = firebaseRef(refName).getAMethodCall("once").getAMethodCall("then").getArgument(0).getALocalSource()
)
or
exists(TypeBackTracker t2 |
result = firebaseSnapshotCallback(refName, t2).backtrack(t2, t)
)
}

FunctionNode firebaseSnapshotCallback(Node refName) {
result = firebaseSnapshotCallback(refName, TypeBackTracker::end())
}

SourceNode firebaseSnapshot(Node refName, TypeTracker t) {
t.start() and
result = firebaseSnapshotCallback(refName).getParameter(0)
or
exists(TypeTracker t2 |
result = firebaseSnapshot(refName, t2).track(t2, t)
)
}

SourceNode firebaseSnapshot(Node refName) {
result = firebaseSnapshot(refName, TypeTracker::end())
}

MethodCallNode firebaseDatabaseRead(Node refName) {
result = firebaseSnapshot(refName).getAMethodCall("val")
}

from Node name
select name, firebaseDatabaseRead(name)

Summary
-------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ Here's a first version of our query:
wsinner > wsouter
select outer, "Whitespace around nested operators contradicts precedence."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/8141155897270480914/>`__. This query is likely to find results on most projects.
This query is likely to find results on most codebases.

The first conjunct of the ``where`` clause restricts ``inner`` to be an operand of ``outer``, the second conjunct binds ``wsinner`` and ``wsouter``, while the last conjunct selects the suspicious cases.

Expand Down Expand Up @@ -143,7 +143,7 @@ Note that our predicate ``operatorWS`` computes the **total** amount of white sp
wsinner > wsouter
select outer, "Whitespace around nested operators contradicts precedence."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/3151720037708691205/>`__. Any results will be refined by our changes to the query.
Any results will be refined by our changes to the query.

Another source of false positives are associative operators: in an expression of the form ``x + y+z``, the first plus is syntactically nested inside the second, since + in Java associates to the left; hence the expression is flagged as suspicious. But since + is associative to begin with, it does not matter which way around the operators are nested, so this is a false positive. To exclude these cases, let us define a new class identifying binary expressions with an associative operator:

Expand Down Expand Up @@ -175,8 +175,6 @@ Now we can extend our query to discard results where the outer and the inner exp
wsinner > wsouter
select outer, "Whitespace around nested operators contradicts precedence."

➤ `See this in the query console on LGTM.com <https://lgtm.com/query/5714614966569401039/>`__.

Notice that we again use ``getOp``, this time to determine whether two binary expressions have the same operator. Running our improved query now finds the Java standard library bug described in the Overview. It also flags up the following suspicious code in `Hadoop HBase <https://hbase.apache.org/>`__:

.. code-block:: java
Expand Down