Skip to content

Commit

Permalink
Java: apply query alert restrictions
Browse files Browse the repository at this point in the history
  • Loading branch information
cklin committed Aug 13, 2024
1 parent aed9c89 commit 29ca5c3
Show file tree
Hide file tree
Showing 60 changed files with 243 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ abstract class Storable extends Call {
abstract Expr getAStore();
}

private module SensitiveSourceFlowConfig implements DataFlow::ConfigSig {
/** Flow configuration for sensitive data flowing into cleartext storage. */
module SensitiveSourceFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }

predicate isSink(DataFlow::Node sink) { sink instanceof CleartextStorageSink }
Expand Down
16 changes: 12 additions & 4 deletions java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ private import semmle.code.java.security.InformationLeak
/**
* One of the `printStackTrace()` overloads on `Throwable`.
*/
private class PrintStackTraceMethod extends Method {
class PrintStackTraceMethod extends Method {
PrintStackTraceMethod() {
this.getDeclaringType()
.getSourceDeclaration()
Expand All @@ -17,7 +17,11 @@ private class PrintStackTraceMethod extends Method {
}
}

private module ServletWriterSourceToPrintStackTraceMethodFlowConfig implements DataFlow::ConfigSig {
/**
* Flow configuration for xss vulnerable writer source flowing to `Throwable.printStackTrace()` on
* a stream that is connected to external output.
*/
module ServletWriterSourceToPrintStackTraceMethodFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof XssVulnerableWriterSourceNode }

predicate isSink(DataFlow::Node sink) {
Expand Down Expand Up @@ -55,7 +59,10 @@ private predicate printWriterOnStringWriter(Expr printWriter, Variable stringWri
)
}

private predicate stackTraceExpr(Expr exception, MethodCall stackTraceString) {
/**
* Holds if `stackTraceString` writes the stack trace from `exception` to a string.
*/
predicate stackTraceExpr(Expr exception, MethodCall stackTraceString) {
exists(Expr printWriter, Variable stringWriterVar, MethodCall printStackCall |
printWriterOnStringWriter(printWriter, stringWriterVar) and
printStackCall.getMethod() instanceof PrintStackTraceMethod and
Expand All @@ -66,7 +73,8 @@ private predicate stackTraceExpr(Expr exception, MethodCall stackTraceString) {
)
}

private module StackTraceStringToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig {
/** Flow configuration for stack trace flowing to http response. */
module StackTraceStringToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { stackTraceExpr(_, src.asExpr()) }

predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ deprecated class UnsafeDeserializationConfig extends TaintTracking::Configuratio
}

/** Tracks flows from remote user input to a deserialization sink. */
private module UnsafeDeserializationConfig implements DataFlow::ConfigSig {
module UnsafeDeserializationConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }

predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink }
Expand Down
1 change: 1 addition & 0 deletions java/ql/src/Likely Bugs/Arithmetic/InformationLoss.ql
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Variable getVariable(Expr dest) {

from DangerousAssignOpExpr a, Expr e, Top v
where
AlertFiltering::filterByLocatable(a) and
e = a.getSource() and
problematicCasting(a.getDest().getType(), e) and
(
Expand Down
2 changes: 2 additions & 0 deletions java/ql/src/Security/CWE/CWE-020/OverlyLargeRange.ql
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* external/cwe/cwe-020
*/

private import semmle.code.java.AlertFiltering
private import semmle.code.java.regex.RegexTreeView::RegexTreeView as TreeView
import codeql.regex.OverlyLargeRangeQuery::Make<TreeView>

Expand All @@ -22,6 +23,7 @@ TreeView::RegExpCharacterClass potentialMisparsedCharClass() {

from TreeView::RegExpCharacterRange range, string reason
where
AlertFiltering::filterByLocation(range.getLocation()) and
problem(range, reason) and
not range.getParent() = potentialMisparsedCharClass()
select range, "Suspicious character range that " + reason + "."
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
import java
import semmle.code.java.security.PathCreation
import semmle.code.java.security.TaintedPathQuery

module TaintedPathFlow = TaintTracking::Global<DataFlow::FilteredConfig<TaintedPathConfig>>;

import TaintedPathFlow::PathGraph

from TaintedPathFlow::PathNode source, TaintedPathFlow::PathNode sink
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-022/ZipSlip.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@

import java
import semmle.code.java.security.ZipSlipQuery

module ZipSlipFlow = TaintTracking::Global<DataFlow::FilteredConfig<ZipSlipConfig>>;

import ZipSlipFlow::PathGraph

from ZipSlipFlow::PathNode source, ZipSlipFlow::PathNode sink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
*/

import semmle.code.java.security.PartialPathTraversalQuery

module PartialPathTraversalFromRemoteFlow =
TaintTracking::Global<DataFlow::FilteredConfig<PartialPathTraversalFromRemoteConfig>>;

import PartialPathTraversalFromRemoteFlow::PathGraph

from
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-074/JndiInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

import java
import semmle.code.java.security.JndiInjectionQuery

module JndiInjectionFlow = TaintTracking::Global<DataFlow::FilteredConfig<JndiInjectionFlowConfig>>;

import JndiInjectionFlow::PathGraph

from JndiInjectionFlow::PathNode source, JndiInjectionFlow::PathNode sink
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-074/XsltInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

import java
import semmle.code.java.security.XsltInjectionQuery

module XsltInjectionFlow = TaintTracking::Global<DataFlow::FilteredConfig<XsltInjectionFlowConfig>>;

import XsltInjectionFlow::PathGraph

from XsltInjectionFlow::PathNode source, XsltInjectionFlow::PathNode sink
Expand Down
12 changes: 12 additions & 0 deletions java/ql/src/Security/CWE/CWE-078/ExecTainted.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,20 @@

import java
import semmle.code.java.security.CommandLineQuery
private import semmle.code.java.dataflow.TaintTracking

module InputToArgumentToExecFlow =
TaintTracking::Global<DataFlow::FilteredConfig<InputToArgumentToExecFlowConfig>>;

import InputToArgumentToExecFlow::PathGraph

predicate execIsTainted(
InputToArgumentToExecFlow::PathNode source, InputToArgumentToExecFlow::PathNode sink, Expr execArg
) {
InputToArgumentToExecFlow::flowPath(source, sink) and
argumentToExec(execArg, sink.getNode())
}

from
InputToArgumentToExecFlow::PathNode source, InputToArgumentToExecFlow::PathNode sink, Expr execArg
where execIsTainted(source, sink, execArg)
Expand Down
1 change: 1 addition & 0 deletions java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ predicate builtFromUncontrolledConcat(Expr expr) {

from StringArgumentToExec argument
where
AlertFiltering::filterByLocatable(argument) and
builtFromUncontrolledConcat(argument) and
not execIsTainted(_, _, argument)
select argument, "Command line is built with string concatenation."
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-079/XSS.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

import java
import semmle.code.java.security.XssQuery

module XssFlow = TaintTracking::Global<DataFlow::FilteredConfig<XssConfig>>;

import XssFlow::PathGraph

from XssFlow::PathNode source, XssFlow::PathNode sink
Expand Down
10 changes: 10 additions & 0 deletions java/ql/src/Security/CWE/CWE-089/SqlTainted.ql
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,18 @@
import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.SqlInjectionQuery

module QueryInjectionFlow =
TaintTracking::Global<DataFlow::FilteredConfig<QueryInjectionFlowConfig>>;

import QueryInjectionFlow::PathGraph

predicate queryIsTaintedBy(
QueryInjectionSink query, QueryInjectionFlow::PathNode source, QueryInjectionFlow::PathNode sink
) {
QueryInjectionFlow::flowPath(source, sink) and sink.getNode() = query
}

from
QueryInjectionSink query, QueryInjectionFlow::PathNode source, QueryInjectionFlow::PathNode sink
where queryIsTaintedBy(query, source, sink)
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-090/LdapInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.LdapInjectionQuery

module LdapInjectionFlow = TaintTracking::Global<DataFlow::FilteredConfig<LdapInjectionFlowConfig>>;

import LdapInjectionFlow::PathGraph

from LdapInjectionFlow::PathNode source, LdapInjectionFlow::PathNode sink
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-094/GroovyInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

import java
import semmle.code.java.security.GroovyInjectionQuery

module GroovyInjectionFlow = TaintTracking::Global<DataFlow::FilteredConfig<GroovyInjectionConfig>>;

import GroovyInjectionFlow::PathGraph

from GroovyInjectionFlow::PathNode source, GroovyInjectionFlow::PathNode sink
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

import java
import semmle.code.java.security.InsecureBeanValidationQuery

module BeanValidationFlow = TaintTracking::Global<DataFlow::FilteredConfig<BeanValidationConfig>>;

import BeanValidationFlow::PathGraph

from BeanValidationFlow::PathNode source, BeanValidationFlow::PathNode sink
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-094/JexlInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

import java
import semmle.code.java.security.JexlInjectionQuery

module JexlInjectionFlow = TaintTracking::Global<DataFlow::FilteredConfig<JexlInjectionConfig>>;

import JexlInjectionFlow::PathGraph

from JexlInjectionFlow::PathNode source, JexlInjectionFlow::PathNode sink
Expand Down
3 changes: 3 additions & 0 deletions java/ql/src/Security/CWE/CWE-094/MvelInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

import java
import semmle.code.java.security.MvelInjectionQuery

module MvelInjectionFlow = TaintTracking::Global<DataFlow::FilteredConfig<MvelInjectionFlowConfig>>;

import MvelInjectionFlow::PathGraph

from MvelInjectionFlow::PathNode source, MvelInjectionFlow::PathNode sink
Expand Down
5 changes: 4 additions & 1 deletion java/ql/src/Security/CWE/CWE-094/SpelInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@

import java
import semmle.code.java.security.SpelInjectionQuery
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking

module SpelInjectionFlow = TaintTracking::Global<DataFlow::FilteredConfig<SpelInjectionConfig>>;

import SpelInjectionFlow::PathGraph

from SpelInjectionFlow::PathNode source, SpelInjectionFlow::PathNode sink
Expand Down
4 changes: 4 additions & 0 deletions java/ql/src/Security/CWE/CWE-094/TemplateInjection.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

import java
import semmle.code.java.security.TemplateInjectionQuery

module TemplateInjectionFlow =
TaintTracking::Global<DataFlow::FilteredConfig<TemplateInjectionFlowConfig>>;

import TemplateInjectionFlow::PathGraph

from TemplateInjectionFlow::PathNode source, TemplateInjectionFlow::PathNode sink
Expand Down
1 change: 1 addition & 0 deletions java/ql/src/Security/CWE/CWE-113/NettyResponseSplitting.ql
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,5 @@ private class InsecureDefaultFullHttpRequestClassInstantiation extends RequestSp
}

from InsecureNettyObjectCreation new
where AlertFiltering::filterByLocatable(new)
select new, new.splittingType() + " vulnerability due to header value verification being disabled."
4 changes: 4 additions & 0 deletions java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

import java
import semmle.code.java.security.ResponseSplittingQuery

module ResponseSplittingFlow =
TaintTracking::Global<DataFlow::FilteredConfig<ResponseSplittingConfig>>;

import ResponseSplittingFlow::PathGraph

from ResponseSplittingFlow::PathNode source, ResponseSplittingFlow::PathNode sink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

import java
import semmle.code.java.security.StaticInitializationVectorQuery

module StaticInitializationVectorFlow =
TaintTracking::Global<DataFlow::FilteredConfig<StaticInitializationVectorConfig>>;

import StaticInitializationVectorFlow::PathGraph

from StaticInitializationVectorFlow::PathNode source, StaticInitializationVectorFlow::PathNode sink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@
*/

import java
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.security.ExternallyControlledFormatStringQuery
import semmle.code.java.StringFormat

module ExternallyControlledFormatStringFlow =
TaintTracking::Global<DataFlow::FilteredConfig<ExternallyControlledFormatStringConfig>>;

import ExternallyControlledFormatStringFlow::PathGraph

from
Expand Down
27 changes: 27 additions & 0 deletions java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,35 @@

import java
import semmle.code.java.dataflow.DataFlow

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.java.dataflow.TaintTracking
.
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.security.StackTraceExposureQuery

private module ServletWriterSourceToPrintStackTraceMethodFlow =
TaintTracking::Global<DataFlow::FilteredConfig<ServletWriterSourceToPrintStackTraceMethodFlowConfig>>;

private predicate printsStackToWriter(MethodCall call) {
exists(PrintStackTraceMethod printStackTrace |
call.getMethod() = printStackTrace and
ServletWriterSourceToPrintStackTraceMethodFlow::flowToExpr(call.getAnArgument())
)
}

predicate printsStackExternally(MethodCall call, Expr stackTrace) {
printsStackToWriter(call) and
call.getQualifier() = stackTrace and
not call.getQualifier() instanceof SuperAccess
}

private module StackTraceStringToHttpResponseSinkFlow =
TaintTracking::Global<DataFlow::FilteredConfig<StackTraceStringToHttpResponseSinkFlowConfig>>;

predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stackTrace) {
exists(MethodCall stackTraceString |
stackTraceExpr(stackTrace, stackTraceString) and
StackTraceStringToHttpResponseSinkFlow::flow(DataFlow::exprNode(stackTraceString), externalExpr)
)
}

from Expr externalExpr, Expr errorInformation
where
printsStackExternally(externalExpr, errorInformation) or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@

import java
import semmle.code.java.security.IntentUriPermissionManipulationQuery
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking

module IntentUriPermissionManipulationFlow =
TaintTracking::Global<DataFlow::FilteredConfig<IntentUriPermissionManipulationConfig>>;

import IntentUriPermissionManipulationFlow::PathGraph

from
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ import java
import semmle.code.java.security.AndroidLocalAuthQuery

from AuthenticationSuccessCallback c
where not exists(c.getAResultUse())
where
AlertFiltering::filterByLocatable(c) and
not exists(c.getAResultUse())
select c, "This authentication callback does not use its result for a cryptographic operation."
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ import java
import semmle.code.java.security.AndroidWebViewCertificateValidationQuery

from OnReceivedSslErrorMethod m
where trustsAllCerts(m)
where
AlertFiltering::filterByLocatable(m) and
trustsAllCerts(m)
select m, "This handler accepts all SSL certificates."
4 changes: 4 additions & 0 deletions java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.security.InsecureTrustManagerQuery

module InsecureTrustManagerFlow =
DataFlow::Global<DataFlow::FilteredConfig<InsecureTrustManagerConfig>>;

import InsecureTrustManagerFlow::PathGraph

from InsecureTrustManagerFlow::PathNode source, InsecureTrustManagerFlow::PathNode sink
Expand Down
Loading

0 comments on commit 29ca5c3

Please sign in to comment.