mirror of
https://github.com/github/codeql.git
synced 2026-05-14 11:19:27 +02:00
Merge remote-tracking branch 'origin/main' into redsun82/just2
This commit is contained in:
@@ -26,10 +26,23 @@ string permissionsForJob(Job job) {
|
||||
"{" + concat(string permission | permission = jobNeedsPermission(job) | permission, ", ") + "}"
|
||||
}
|
||||
|
||||
predicate jobHasPermissions(Job job) {
|
||||
exists(job.getPermissions())
|
||||
or
|
||||
exists(job.getEnclosingWorkflow().getPermissions())
|
||||
or
|
||||
// The workflow is reusable and cannot be triggered in any other way; check callers
|
||||
exists(ReusableWorkflow r | r = job.getEnclosingWorkflow() |
|
||||
not exists(Event e | e = r.getOn().getAnEvent() | e.getName() != "workflow_call") and
|
||||
forall(Job caller | caller = job.getEnclosingWorkflow().(ReusableWorkflow).getACaller() |
|
||||
jobHasPermissions(caller)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
from Job job, string permissions
|
||||
where
|
||||
not exists(job.getPermissions()) and
|
||||
not exists(job.getEnclosingWorkflow().getPermissions()) and
|
||||
not jobHasPermissions(job) and
|
||||
// exists a trigger event that is not a workflow_call
|
||||
exists(Event e |
|
||||
e = job.getATriggerEvent() and
|
||||
|
||||
4
actions/ql/src/change-notes/2026-04-02-permissions.md
Normal file
4
actions/ql/src/change-notes/2026-04-02-permissions.md
Normal file
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The query `actions/missing-workflow-permissions` no longer produces false positive results on reusable workflows where all callers set permissions.
|
||||
9
actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms11.yml
vendored
Normal file
9
actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms11.yml
vendored
Normal file
@@ -0,0 +1,9 @@
|
||||
on:
|
||||
workflow_call:
|
||||
|
||||
jobs:
|
||||
build:
|
||||
name: Build and test
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/deploy-pages
|
||||
11
actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms12.yml
vendored
Normal file
11
actions/ql/test/query-tests/Security/CWE-275/.github/workflows/perms12.yml
vendored
Normal file
@@ -0,0 +1,11 @@
|
||||
on:
|
||||
workflow_dispatch:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
id-token: write
|
||||
pages: write
|
||||
|
||||
jobs:
|
||||
call-workflow:
|
||||
uses: ./.github/workflows/perms11.yml
|
||||
@@ -7,10 +7,12 @@ ql/cpp/ql/src/Diagnostics/ExtractedFiles.ql
|
||||
ql/cpp/ql/src/Diagnostics/ExtractionWarnings.ql
|
||||
ql/cpp/ql/src/Diagnostics/FailedExtractorInvocations.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Arithmetic/BadAdditionOverflowCheck.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Arithmetic/SignedOverflowCheck.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Format/SnprintfOverflow.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Format/WrongNumberOfFormatArguments.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Memory Management/AllocaInLoop.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Memory Management/PointerOverflow.ql
|
||||
ql/cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql
|
||||
@@ -28,6 +30,7 @@ ql/cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
|
||||
ql/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql
|
||||
ql/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql
|
||||
ql/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql
|
||||
ql/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql
|
||||
ql/cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql
|
||||
ql/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql
|
||||
ql/cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 8.1
|
||||
* @precision medium
|
||||
* @precision high
|
||||
* @id cpp/integer-multiplication-cast-to-long
|
||||
* @tags reliability
|
||||
* security
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* @kind problem
|
||||
* @problem.severity error
|
||||
* @security-severity 7.5
|
||||
* @precision medium
|
||||
* @precision high
|
||||
* @id cpp/wrong-type-format-argument
|
||||
* @tags reliability
|
||||
* correctness
|
||||
|
||||
@@ -14,6 +14,9 @@ function may behave unpredictably.</p>
|
||||
<p>This may indicate a misspelled function name, or that the required header containing
|
||||
the function declaration has not been included.</p>
|
||||
|
||||
<p>Note: This query is not compatible with <code>build mode: none</code> databases, and produces
|
||||
no results on those databases.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Provide an explicit declaration of the function before invoking it.</p>
|
||||
@@ -26,4 +29,4 @@ the function declaration has not been included.</p>
|
||||
<references>
|
||||
<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/DCL31-C.+Declare+identifiers+before+using+them">DCL31-C. Declare identifiers before using them</a></li>
|
||||
</references>
|
||||
</qhelp>
|
||||
</qhelp>
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
* may lead to unpredictable behavior.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @precision medium
|
||||
* @precision high
|
||||
* @id cpp/implicit-function-declaration
|
||||
* @tags correctness
|
||||
* maintainability
|
||||
@@ -17,6 +17,11 @@ import TooFewArguments
|
||||
import TooManyArguments
|
||||
import semmle.code.cpp.commons.Exclusions
|
||||
|
||||
/*
|
||||
* This query is not compatible with build mode: none databases, and produces
|
||||
* no results on those databases.
|
||||
*/
|
||||
|
||||
predicate locInfo(Locatable e, File file, int line, int col) {
|
||||
e.getFile() = file and
|
||||
e.getLocation().getStartLine() = line and
|
||||
@@ -39,6 +44,7 @@ predicate isCompiledAsC(File f) {
|
||||
from FunctionDeclarationEntry fdeIm, FunctionCall fc
|
||||
where
|
||||
isCompiledAsC(fdeIm.getFile()) and
|
||||
not any(Compilation c).buildModeNone() and
|
||||
not isFromMacroDefinition(fc) and
|
||||
fdeIm.isImplicit() and
|
||||
sameLocation(fdeIm, fc) and
|
||||
|
||||
@@ -79,9 +79,7 @@ private predicate hasZeroParamDecl(Function f) {
|
||||
|
||||
// True if this file (or header) was compiled as a C file
|
||||
private predicate isCompiledAsC(File f) {
|
||||
f.compiledAsC()
|
||||
or
|
||||
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
|
||||
exists(File src | src.compiledAsC() | src.getAnIncludedFile*() = f)
|
||||
}
|
||||
|
||||
predicate mistypedFunctionArguments(FunctionCall fc, Function f, Parameter p) {
|
||||
|
||||
@@ -28,9 +28,7 @@ private predicate hasZeroParamDecl(Function f) {
|
||||
|
||||
/* Holds if this file (or header) was compiled as a C file. */
|
||||
private predicate isCompiledAsC(File f) {
|
||||
f.compiledAsC()
|
||||
or
|
||||
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
|
||||
exists(File src | src.compiledAsC() | src.getAnIncludedFile*() = f)
|
||||
}
|
||||
|
||||
/** Holds if `fc` is a call to `f` with too few arguments. */
|
||||
|
||||
@@ -19,9 +19,7 @@ private predicate hasZeroParamDecl(Function f) {
|
||||
|
||||
// True if this file (or header) was compiled as a C file
|
||||
private predicate isCompiledAsC(File f) {
|
||||
f.compiledAsC()
|
||||
or
|
||||
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
|
||||
exists(File src | src.compiledAsC() | src.getAnIncludedFile*() = f)
|
||||
}
|
||||
|
||||
predicate tooManyArguments(FunctionCall fc, Function f) {
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 7.8
|
||||
* @precision medium
|
||||
* @precision high
|
||||
* @tags reliability
|
||||
* security
|
||||
* external/cwe/cwe-190
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The "Implicit function declaration" (`cpp/implicit-function-declaration`) query no longer produces results on `build mode: none` databases. These results were found to be very noisy and fundamentally imprecise in this mode.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The "Comparison of narrow type with wide type in loop condition" (`cpp/comparison-with-wider-type`) query has been upgraded to `high` precision. This query will now run in the default code scanning suite.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The "Implicit function declaration" (`cpp/implicit-function-declaration`) query has been upgraded to `high` precision.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The "Multiplication result converted to larger type" (`cpp/integer-multiplication-cast-to-long`) query has been upgraded to `high` precision. This query will now run in the default code scanning suite.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The "Wrong type of arguments to formatting function" (`cpp/wrong-type-format-argument`) query has been upgraded to `high` precision. This query will now run in the default code scanning suite.
|
||||
@@ -191,3 +191,21 @@ class RouteHandlerLimitedByRateLimiterFlexible extends RateLimitingMiddleware in
|
||||
private class FastifyRateLimiter extends RateLimitingMiddleware {
|
||||
FastifyRateLimiter() { this = DataFlow::moduleImport("fastify-rate-limit") }
|
||||
}
|
||||
|
||||
/**
|
||||
* An options object with a `rateLimit` config passed to a Fastify shorthand route method,
|
||||
* such as `fastify.post('/path', { config: { rateLimit: { ... } } }, handler)`.
|
||||
*/
|
||||
private class FastifyPerRouteRateLimit extends RateLimitingMiddleware {
|
||||
FastifyPerRouteRateLimit() {
|
||||
exists(Fastify::RouteSetup setup |
|
||||
not setup.getMethodName() = ["route", "addHook"] and
|
||||
setup.getNumArgument() >= 3 and
|
||||
this.flowsTo(setup.getArgument(1))
|
||||
|
|
||||
exists(this.getAPropertySource("config").getAPropertySource("rateLimit"))
|
||||
or
|
||||
exists(this.getAPropertySource("rateLimit"))
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The query `js/missing-rate-limiting` now takes Fastify per-route
|
||||
rate limiting into account.
|
||||
@@ -9,3 +9,4 @@
|
||||
| tst.js:64:25:64:63 | functio ... req); } | This route handler performs $@, but is not rate-limited. | tst.js:64:46:64:60 | verifyUser(req) | authorization |
|
||||
| tst.js:76:25:76:53 | catchAs ... ndler1) | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
|
||||
| tst.js:88:24:88:40 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
|
||||
| tst.js:112:28:112:44 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
|
||||
|
||||
@@ -88,3 +88,25 @@ const fastifyApp = require('fastify')();
|
||||
fastifyApp.get('/foo', expensiveHandler1); // $ Alert
|
||||
fastifyApp.register(require('fastify-rate-limit'));
|
||||
fastifyApp.get('/bar', expensiveHandler1);
|
||||
|
||||
// Fastify per-route rate limiting via config.rateLimit
|
||||
const fastifyApp2 = require('fastify')();
|
||||
fastifyApp2.register(require('@fastify/rate-limit'));
|
||||
|
||||
fastifyApp2.post('/login', {
|
||||
config: {
|
||||
rateLimit: {
|
||||
max: 3,
|
||||
timeWindow: '1 minute'
|
||||
}
|
||||
}
|
||||
}, expensiveHandler1); // OK - has per-route rateLimit config
|
||||
|
||||
fastifyApp2.post('/signup', {
|
||||
rateLimit: {
|
||||
max: 5,
|
||||
timeWindow: '1 minute'
|
||||
}
|
||||
}, expensiveHandler1); // OK - has per-route rateLimit directly in options
|
||||
|
||||
fastifyApp2.post('/other', expensiveHandler1); // $ Alert - no rate limiting
|
||||
|
||||
@@ -12,10 +12,10 @@
|
||||
*/
|
||||
|
||||
import python
|
||||
private import LegacyPointsTo
|
||||
private import semmle.python.ApiGraphs
|
||||
|
||||
predicate originIsLocals(ControlFlowNodeWithPointsTo n) {
|
||||
n.pointsTo(_, _, Value::named("locals").getACall())
|
||||
predicate originIsLocals(ControlFlowNode n) {
|
||||
API::builtin("locals").getReturn().getAValueReachableFromSource().asCfgNode() = n
|
||||
}
|
||||
|
||||
predicate modification_of_locals(ControlFlowNode f) {
|
||||
@@ -37,5 +37,8 @@ where
|
||||
// in module level scope `locals() == globals()`
|
||||
// see https://docs.python.org/3/library/functions.html#locals
|
||||
// FP report in https://github.com/github/codeql/issues/6674
|
||||
not a.getScope() instanceof ModuleScope
|
||||
not a.getScope() instanceof Module and
|
||||
// in class level scope `locals()` reflects the class namespace,
|
||||
// so modifications do take effect.
|
||||
not a.getScope() instanceof Class
|
||||
select a, "Modification of the locals() dictionary will have no effect on the local variables."
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
*/
|
||||
|
||||
import python
|
||||
private import LegacyPointsTo
|
||||
private import semmle.python.ApiGraphs
|
||||
|
||||
predicate typing_import(ImportingStmt is) {
|
||||
exists(Module m |
|
||||
@@ -34,11 +34,7 @@ predicate unique_yield(Stmt s) {
|
||||
/** Holds if `contextlib.suppress` may be used in the same scope as `s` */
|
||||
predicate suppression_in_scope(Stmt s) {
|
||||
exists(With w |
|
||||
w.getContextExpr()
|
||||
.(Call)
|
||||
.getFunc()
|
||||
.(ExprWithPointsTo)
|
||||
.pointsTo(Value::named("contextlib.suppress")) and
|
||||
w.getContextExpr() = API::moduleImport("contextlib").getMember("suppress").getACall().asExpr() and
|
||||
w.getScope() = s.getScope()
|
||||
)
|
||||
}
|
||||
|
||||
@@ -12,11 +12,49 @@
|
||||
*/
|
||||
|
||||
import python
|
||||
private import LegacyPointsTo
|
||||
private import semmle.python.dataflow.new.internal.DataFlowDispatch
|
||||
private import semmle.python.dataflow.new.internal.Builtins
|
||||
private import semmle.python.ApiGraphs
|
||||
|
||||
from Call call, ClassValue ex
|
||||
/**
|
||||
* Holds if `cls` is a user-defined exception class, i.e. it transitively
|
||||
* extends one of the builtin exception base classes.
|
||||
*/
|
||||
predicate isUserDefinedExceptionClass(Class cls) {
|
||||
cls.getABase() =
|
||||
API::builtin(["BaseException", "Exception"]).getAValueReachableFromSource().asExpr()
|
||||
or
|
||||
isUserDefinedExceptionClass(getADirectSuperclass(cls))
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the name of a builtin exception class.
|
||||
*/
|
||||
string getBuiltinExceptionName() {
|
||||
result = Builtins::getBuiltinName() and
|
||||
(
|
||||
result.matches("%Error") or
|
||||
result.matches("%Exception") or
|
||||
result.matches("%Warning") or
|
||||
result =
|
||||
["GeneratorExit", "KeyboardInterrupt", "StopIteration", "StopAsyncIteration", "SystemExit"]
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `call` is an instantiation of an exception class.
|
||||
*/
|
||||
predicate isExceptionInstantiation(Call call) {
|
||||
exists(Class cls |
|
||||
classTracker(cls).asExpr() = call.getFunc() and
|
||||
isUserDefinedExceptionClass(cls)
|
||||
)
|
||||
or
|
||||
call.getFunc() = API::builtin(getBuiltinExceptionName()).getAValueReachableFromSource().asExpr()
|
||||
}
|
||||
|
||||
from Call call
|
||||
where
|
||||
call.getFunc().(ExprWithPointsTo).pointsTo(ex) and
|
||||
ex.getASuperType() = ClassValue::exception() and
|
||||
isExceptionInstantiation(call) and
|
||||
exists(ExprStmt s | s.getValue() = call)
|
||||
select call, "Instantiating an exception, but not raising it, has no effect."
|
||||
|
||||
@@ -12,10 +12,12 @@
|
||||
*/
|
||||
|
||||
import python
|
||||
private import LegacyPointsTo
|
||||
private import semmle.python.ApiGraphs
|
||||
|
||||
from CallNode call, string name
|
||||
where call.getFunction().(ControlFlowNodeWithPointsTo).pointsTo(Value::siteQuitter(name))
|
||||
where
|
||||
name = ["exit", "quit"] and
|
||||
call = API::builtin(name).getACall().asCfgNode()
|
||||
select call,
|
||||
"The '" + name +
|
||||
"' site.Quitter object may not exist if the 'site' module is not loaded or is modified."
|
||||
|
||||
@@ -174,3 +174,9 @@ def assert_ok(seq):
|
||||
# False positive. ODASA-8042. Fixed in PR #2401.
|
||||
class false_positive:
|
||||
e = (x for x in [])
|
||||
|
||||
# In class-level scope `locals()` reflects the class namespace,
|
||||
# so modifications do take effect.
|
||||
class MyClass:
|
||||
locals()['x'] = 43 # OK
|
||||
y = x
|
||||
|
||||
67
python/scripts/create-extractor-pack.sh
Executable file
67
python/scripts/create-extractor-pack.sh
Executable file
@@ -0,0 +1,67 @@
|
||||
#!/bin/bash
|
||||
#
|
||||
# Build a local Python extractor pack from source.
|
||||
#
|
||||
# Usage with the CodeQL CLI (run from the repository root):
|
||||
#
|
||||
# codeql database create <db> -l python -s <src> --search-path .
|
||||
# codeql test run --search-path . python/ql/test/<test-dir>
|
||||
#
|
||||
set -eux
|
||||
|
||||
if [[ "$OSTYPE" == "linux-gnu"* ]]; then
|
||||
platform="linux64"
|
||||
elif [[ "$OSTYPE" == "darwin"* ]]; then
|
||||
platform="osx64"
|
||||
else
|
||||
echo "Unknown OS"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
cd "$(dirname "$0")/.."
|
||||
|
||||
# Build the tsg-python Rust binary
|
||||
(cd extractor/tsg-python && cargo build --release)
|
||||
tsg_bin="extractor/tsg-python/target/release/tsg-python"
|
||||
|
||||
# Generate python3src.zip from the Python extractor source.
|
||||
# make_zips.py creates the zip in the source directory and then copies it to the
|
||||
# given output directory. We use a temporary directory to avoid a same-file copy
|
||||
# error, then move the zip back.
|
||||
tmpdir=$(mktemp -d)
|
||||
trap 'rm -rf "$tmpdir"' EXIT
|
||||
(cd extractor && python3 make_zips.py "$tmpdir")
|
||||
cp "$tmpdir/python3src.zip" extractor/python3src.zip
|
||||
|
||||
# Assemble the extractor pack
|
||||
rm -rf extractor-pack
|
||||
mkdir -p extractor-pack/tools/${platform}
|
||||
|
||||
# Root-level metadata and schema files
|
||||
cp codeql-extractor.yml extractor-pack/
|
||||
cp ql/lib/semmlecode.python.dbscheme extractor-pack/
|
||||
cp ql/lib/semmlecode.python.dbscheme.stats extractor-pack/
|
||||
|
||||
# Python extractor engine files (into tools/)
|
||||
cp extractor/python_tracer.py extractor-pack/tools/
|
||||
cp extractor/index.py extractor-pack/tools/
|
||||
cp extractor/setup.py extractor-pack/tools/
|
||||
cp extractor/convert_setup.py extractor-pack/tools/
|
||||
cp extractor/get_venv_lib.py extractor-pack/tools/
|
||||
cp extractor/imp.py extractor-pack/tools/
|
||||
cp extractor/LICENSE-PSF.md extractor-pack/tools/
|
||||
cp extractor/python3src.zip extractor-pack/tools/
|
||||
cp -r extractor/data extractor-pack/tools/
|
||||
|
||||
# Shell tool scripts (autobuild, pre-finalize, lgtm-scripts)
|
||||
cp tools/autobuild.sh extractor-pack/tools/
|
||||
cp tools/autobuild.cmd extractor-pack/tools/
|
||||
cp tools/pre-finalize.sh extractor-pack/tools/
|
||||
cp tools/pre-finalize.cmd extractor-pack/tools/
|
||||
cp -r tools/lgtm-scripts extractor-pack/tools/
|
||||
|
||||
# Downgrades
|
||||
cp -r downgrades extractor-pack/
|
||||
|
||||
# Platform-specific Rust binary
|
||||
cp "${tsg_bin}" extractor-pack/tools/${platform}/tsg-python
|
||||
Reference in New Issue
Block a user