Java: Detecting flow through throw - catch statements

Abierto
#19,336 2 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
30/100
Tipo de issue
Error
Claridad
Necesita aclaración
Estado de actividad
Estancado
Stack tecnológico
java

Línea de trabajo

Comienza con qlpack.yml y codeql-pack.lock.yml; después, inspecciona los puntos de entrada isSource, isSink e isAdditionalFlowStep de la query usando el toy Java example. Ejecuta la query con la versión de CLI indicada y verifica si se detecta el flujo throw-to-catch; el trabajo estará terminado cuando se identifique la dependencia o el comportamiento de la query responsable de que falte el flujo.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

question

Hello,

I'm having a confusing issue trying to track the data flow from a throw to a catch statement. I had similar queries working just fine. However, I was messing around with my qlpack last week and I think I may have messed something up. To give you some context, I am using CLI version 2.20.3 and here is my

qlpack.yml

---
library: false
warnOnImplicitThis: false
name: custom-codeql-queries
version: 0.0.1
dependencies:
  # codeql/java-queries: ^0.8.9
  codeql/java-all: 6.1.0
libraries:
  - name: SensitiveInfo
    path: /SensitiveInfo
dataExtensions:
  - SensitiveInfo/*.yml

Along with my codeql-pack.lock.yml

---
lockVersion: 1.0.0
dependencies:
  codeql/dataflow:
    version: 1.1.9
  codeql/java-all:
    version: 6.1.0
  codeql/mad:
    version: 1.0.15
  codeql/rangeanalysis:
    version: 1.0.15
  codeql/regex:
    version: 1.0.15
  codeql/ssa:
    version: 1.0.15
  codeql/threat-models:
    version: 1.0.15
  codeql/tutorial:
    version: 1.0.15
  codeql/typeflow:
    version: 1.0.15
  codeql/typetracking:
    version: 1.0.15
  codeql/util:
    version: 2.0.2
  codeql/xml:
    version: 1.0.15
compiled: false

I don't fully understand all of the dependency versions needed to work with my CLI version so I would appriacte it if someones could point out if this looks wrong.

On to my specific issue.

I am trying to detect this toy example.

import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.ejb.EJBException;
import java.io.IOException;

public class BAD_AuthenticationFailureServlet extends HttpServlet {
    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
        String username = request.getParameter("username");
        String password = request.getParameter("password");
        try {
            authenticateUser(username, password);
        } catch (EJBException e) {
            response.getWriter().write("Authentication failed: " + e.getMessage());
        }
    }

    private void authenticateUser(String username, String password) throws EJBException {
        if (username == null || password == null) {
            throw new EJBException("Username or password is null. Provided username: " + username + ", password: " + password);
        }
        throw new EJBException("Invalid credentials provided for user " + username + ". Attempted password: " + password);
    }
}

With this query

/**
 * @name CWE-550: Exposure of sensitive information through servlet responses
 * @description Detects flows of the `password` parameter into EJBException messages that are written back to HTTP clients.
 * @kind path-problem
 * @problem.severity warning
 * @id java/servlet-info-exposure/550
 * @tags security
 *       external/cwe/cwe-550
 *       external/cwe/cwe-200
 * @cwe CWE-550
 */

 import java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.DataFlow
 
 // Define flow states
 private newtype MyFlowState =
   State1() or
   State2() or
   State3()
 
 module ServerGeneratedErrorMessageConfig implements DataFlow::StateConfigSig {
   class FlowState = MyFlowState;
 
     // Source: the `password` servlet parameter
   predicate isSource(DataFlow::Node source, FlowState state) {
     state instanceof State1 and
     exists(VarAccess va |
       va.getVariable().getName() = "password" and
       source.asExpr() = va
     )
   }
 
 
   // Sink: explicit HTTP response sinks that expose the message
   predicate isSink(DataFlow::Node sink, FlowState state) {
     state instanceof State3 and (
       // response.getWriter().write(msg)
       exists(MethodCall mc |
         mc.getMethod().getName() = "write" and
         exists(MethodCall getWriter |
           getWriter.getMethod().getName() = "getWriter" and
           getWriter = mc.getQualifier() and
           getWriter.getQualifier().getType().(RefType).hasQualifiedName("javax.servlet.http", "HttpServletResponse")
         ) and
         mc.getArgument(0) = sink.asExpr()
       )
     )
   }
 
   // Transitions between flow states
   predicate isAdditionalFlowStep(
     DataFlow::Node node1, FlowState state1,
     DataFlow::Node node2, FlowState state2
   ) {
     // State1->State2: flows into EJBException constructor
     state1 instanceof State1 and state2 instanceof State2 and
     exists(ConstructorCall cc |
       cc.getAnArgument() = node1.asExpr() and
       cc.getConstructor().getDeclaringType().(RefType)
         .getASupertype+().hasQualifiedName("javax.ejb", "EJBException") and
       node2.asExpr() = cc
     )
     or
     // State2->State3: flows from throw to catch capturing e or e.getMessage()
     state1 instanceof State2 and state2 instanceof State3 and
     exists(ThrowStmt t, CatchClause cc, Expr use |
       t.getExpr() = node1.asExpr() and
       cc.getEnclosingCallable() = t.getEnclosingCallable() and
       (
         use = cc.getVariable().getAnAccess()
         or
         exists(MethodCall mc2 |
           mc2.getQualifier() = cc.getVariable().getAnAccess() and
           mc2.getMethod().getName() = "getMessage" and
           use = mc2
         )
       ) and
       node2.asExpr() = use
     )
   }
 }
 
 module SensitiveInfoInErrorMsgFlow =
   TaintTracking::GlobalWithState<ServerGeneratedErrorMessageConfig>;
 import SensitiveInfoInErrorMsgFlow::PathGraph
 
 from SensitiveInfoInErrorMsgFlow::PathNode source, SensitiveInfoInErrorMsgFlow::PathNode sink
 where SensitiveInfoInErrorMsgFlow::flowPath(source, sink)
 select sink, source, sink,
   "CWE-550: password parameter is exposed via server error message."

I have confirmed that the isSource and isSink do work. So the issue is likely in the flow between them. I have used a few queries in the past that follow this similar structure. However, they are also missing some cases randomly at the moment. Which is leading me to believe it has something to do with my qlpack.

As always, thank you for the help.

Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 10 h
PR fusionados (30 d)
134

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de github/codeql

Todos los issues de github/codeql

Issues similares

Más issues de DevTools

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.