CVE-2025-53622-query.ql

/**
 * @name Path traversal in DSpace import operations without validation
 * @description File paths from external sources are used directly in file operations without proper validation, potentially allowing directory traversal attacks
 * @problem.severity error
 * @security-severity 7.5
 * @precision high
 * @tags security
 * @kind path-problem
 * @id java/dspace-path-traversal-unvalidated
 */
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking
private import semmle.code.java.dataflow.ExternalFlow

class Source extends DataFlow::Node {
  Source() {
    // File path parameters in vulnerable methods (focused on actual target methods)
    exists(Parameter p |
      p.getType() instanceof TypeString and
      p.getCallable().getDeclaringType().getName() = "ItemImportServiceImpl" and
      // Focus on the exact methods we need to hit
      (p.getCallable().getName() = "processContentsFile" and p.getName() = "sFilePath") or
      (p.getCallable().getName() = "processContentFileEntry" and p.getName() = "fileName") or
      (p.getCallable().getName() = "addDCValue" and p.getName().matches("%path%")) or
      (p.getCallable().getName() = "addDCValue" and p.getName().matches("%file%"))
    |
      this.asParameter() = p
    ) or
    // Bitstream getName() calls in getFile method
    exists(MethodCall mc |
      mc.getMethod().getName() = "getName" and
      mc.getMethod().getDeclaringType().getName().matches("%Bitstream%") and
      mc.getEnclosingCallable().getName() = "getFile" and
      mc.getEnclosingCallable().getDeclaringType().getName() = "DSBitStoreService"
    |
      this.asExpr() = mc
    ) or
    // String operations constructing file paths in target methods
    exists(AddExpr ae |
      ae.getEnclosingCallable().getName() in ["processContentsFile", "processContentFileEntry", "getFile", "addDCValue"] and
      // File path construction patterns
      (ae.getAnOperand().(StringLiteral).getValue().matches("%/%") or
       ae.getAnOperand().(FieldRead).getField().getName() = "separatorChar")
    |
      this.asExpr() = ae
    ) or
    // Variable reads of potentially tainted file path variables
    exists(VarAccess va |
      va.getEnclosingCallable().getName() in ["processContentsFile", "processContentFileEntry", "getFile", "addDCValue"] and
      (va.getVariable().getName() = "fileName" or
       va.getVariable().getName() = "sFilePath" or
       va.getVariable().getName() = "fullpath" or
       va.getVariable().getName().matches("%path%"))
    |
      this.asExpr() = va
    )
  }
}

class Sink extends DataFlow::Node {
  Sink() {
    // Method calls that perform file operations in target methods
    exists(MethodCall mc |
      mc.getEnclosingCallable().getName() in ["processContentsFile", "processContentFileEntry", "getFile", "addDCValue"] and
      (
        // registerBitstream calls in ItemImportServiceImpl
        (mc.getMethod().getName() = "registerBitstream" and
         mc.getMethod().getDeclaringType().getName().matches("%ItemImportService%")) or
        // File constructor calls
        (mc.getMethod().getName() = "File" and
         mc.getMethod().getDeclaringType().getName() = "File") or
        // FileInputStream, FileOutputStream etc.
        (mc.getMethod().getName().matches("File%Stream") and
         mc.getMethod().getDeclaringType().getName().matches("%File%Stream")) or
        // File operations like createNewFile, exists, etc.
        (mc.getMethod().getDeclaringType().getName() = "File" and
         mc.getMethod().getName() in ["createNewFile", "exists", "delete", "mkdir", "mkdirs"])
      )
    |
      this.asExpr() = mc.getAnArgument()
    ) or
    // Return statements in getFile method (vulnerable pattern)
    exists(ReturnStmt ret |
      ret.getEnclosingCallable().getName() = "getFile" and
      ret.getEnclosingCallable().getDeclaringType().getName() = "DSBitStoreService"
    |
      this.asExpr() = ret.getResult()
    ) or
    // Assignment to file path variables before operations
    exists(Assignment assign |
      assign.getEnclosingCallable().getName() in ["processContentsFile", "processContentFileEntry", "getFile", "addDCValue"] and
      (assign.getDest().(VarAccess).getVariable().getName() = "fullpath" or
       assign.getDest().(VarAccess).getVariable().getName().matches("%path%"))
    |
      this.asExpr() = assign.getSource()
    )
  }
}

// Detect path validation - this is the critical sanitizer added in the fix
class PathValidator extends DataFlow::Node {
  PathValidator() {
    // Direct validateFilePath method calls (the primary fix)
    exists(MethodCall mc |
      mc.getMethod().getName() = "validateFilePath" and
      mc.getMethod().getDeclaringType().getName() = "ItemImportServiceImpl"
    |
      this.asExpr() = mc.getAnArgument()
    ) or
    // The actual validation pattern from the fix: canonical path with startsWith check
    exists(MethodCall canonical, MethodCall startsWith |
      canonical.getMethod().getName() = "getCanonicalPath" and
      startsWith.getMethod().getName() = "startsWith" and
      canonical.getEnclosingCallable() = startsWith.getEnclosingCallable() and
      // Make sure this is in the context of a conditional check
      exists(IfStmt ifStmt |
        ifStmt.getCondition().getAChildExpr*() = startsWith
      )
    |
      this.asExpr() = canonical.getQualifier()
    ) or
    // Path normalization with subsequent startsWith validation (DSBitStoreService fix)
    exists(MethodCall normalize, MethodCall startsWith |
      normalize.getMethod().getName() = "normalize" and
      startsWith.getMethod().getName() = "startsWith" and
      normalize.getEnclosingCallable() = startsWith.getEnclosingCallable() and
      // Make sure this is in the context of a conditional check
      exists(IfStmt ifStmt |
        ifStmt.getCondition().getAChildExpr*() = startsWith
      )
    |
      this.asExpr() = normalize.getQualifier()
    )
  }
}

module MyPathConfig implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node source) {
    source instanceof Source
  }

  predicate isSink(DataFlow::Node sink) {
    sink instanceof Sink
  }

  predicate isBarrier(DataFlow::Node sanitizer) {
    sanitizer instanceof PathValidator
  }

  predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
    // String concatenation flow
    exists(AddExpr ae |
      n1.asExpr() = ae.getAnOperand() and
      n2.asExpr() = ae
    ) or
    // Variable assignments
    exists(Assignment assign |
      n1.asExpr() = assign.getSource() and
      n2.asExpr() = assign.getDest()
    ) or
    // Method call argument flow
    exists(MethodCall mc, Parameter p |
      n1.asExpr() = mc.getAnArgument() and
      n2.asParameter() = p and
      mc.getMethod().getParameter(_) = p
    ) or
    // File object construction
    exists(ConstructorCall cc |
      cc.getConstructedType().getName() = "File" and
      n1.asExpr() = cc.getAnArgument() and
      n2.asExpr() = cc
    ) or
    // StringBuilder/StringBuffer operations
    exists(MethodCall mc |
      (mc.getMethod().getName() = "append" or mc.getMethod().getName() = "toString") and
      n1.asExpr() = mc.getQualifier() and
      n2.asExpr() = mc
    )
  }
}

module MyPathFlow = TaintTracking::Global<MyPathConfig>;
import MyPathFlow::PathGraph

from
  MyPathFlow::PathNode source,
  MyPathFlow::PathNode sink
where
  MyPathFlow::flowPath(source, sink) and
  // Only report flows in the target methods we want to detect
  exists(Method m |
    m.getName() in ["processContentsFile", "processContentFileEntry", "getFile", "addDCValue"] and
    (sink.getNode().asExpr().getEnclosingCallable() = m or
     source.getNode().asParameter().getCallable() = m or
     source.getNode().asExpr().getEnclosingCallable() = m)
  ) and
  // CRITICAL: Exclude flows where any path validation occurs in the same method or calling context
  not exists(MethodCall validateCall |
    (validateCall.getMethod().getName() = "validateFilePath" or
     (validateCall.getMethod().getName() = "getCanonicalPath" and
      exists(MethodCall startsWithCall |
        startsWithCall.getMethod().getName() = "startsWith" and
        startsWithCall.getEnclosingCallable() = validateCall.getEnclosingCallable()
      )) or
     (validateCall.getMethod().getName() = "normalize" and
      exists(MethodCall startsWithCall |
        startsWithCall.getMethod().getName() = "startsWith" and
        startsWithCall.getEnclosingCallable() = validateCall.getEnclosingCallable()
      ))) and
    (validateCall.getEnclosingCallable() = sink.getNode().asExpr().getEnclosingCallable() or
     // Also check if validation happens in a method that's called from the sink method
     exists(MethodCall callToValidationMethod |
       callToValidationMethod.getEnclosingCallable() = sink.getNode().asExpr().getEnclosingCallable() and
       callToValidationMethod.getMethod() = validateCall.getEnclosingCallable()
     ))
  ) and
  // Additional check: exclude flows where the validateFilePath method exists in the same class (fixed version)
  not exists(Method validateMethod |
    validateMethod.getName() = "validateFilePath" and
    validateMethod.getDeclaringType() = sink.getNode().asExpr().getEnclosingCallable().getDeclaringType()
  )
select
  sink.getNode(),
  source,
  sink,
  "Unvalidated file path from $@ is used in file operation without proper validation, potentially allowing directory traversal",
  source.getNode(),
  "external source"