Answers & Feedback - GitHub Security Lab CTF 4: CodeQL and chill - The Java edition

This page includes a reference solution written by CTF reviewers during the contest. There are many correct ways to solve this challenge; this is one approach.

Note that several participants' solutions and their associated community feedback are also available in our forum.

Complete reference answer

/**
 * @name Insecure Bean validation
 * @description User-controlled data may be evaluated as Java EL expressions, leading to arbitrary code execution.
 * @kind path-problem
 * @problem.severity error
 * @precision high
 * @id java/ctf/unsafe-eval
 * @tags security
 *       external/cwe/cwe-094
 */

import java
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph

// only for Step 1.4
// import DataFlow::PartialPathGraph
/* Step 1.1 */
/** The generic interface `javax.validation.ConstraintValidator`. */
class TypeConstraintValidator extends Interface {
  TypeConstraintValidator() { this.hasQualifiedName("javax.validation", "ConstraintValidator") }

  /** Gets a method named `isValid` declared in this interface. */
  Method getIsValidMethod() {
    result.getDeclaringType() = this and
    result.hasName("isValid")
  }
}

/**
 * An implementation of the method named `isValid` from the
 * interface `javax.validation.ConstraintValidator`.
 */
class ConstraintValidatorIsValidMethod extends Method {
  ConstraintValidatorIsValidMethod() {
    this.overridesOrInstantiates*(any(TypeConstraintValidator t).getIsValidMethod())
  }
}

/**
 * The first parameter of a method that implements `ConstraintValidator.isValid`,
 * where the method is used to validate a field whose value comes from user input.
 */
class BeanValidationSource extends DataFlow::Node {
  BeanValidationSource() {
    exists(ConstraintValidatorIsValidMethod isValidMethod |
      // This source is the first parameter of the `isValid` method
      this.asParameter() = isValidMethod.getParameter(0) and
      // which must be present in the source code
      isValidMethod.fromSource() and
      // Bonus step: and must be used to validate user-controlled data
      Bonus::validatesUserControlledBeanProperty(isValidMethod, _, _, _)
    )
  }
}

/* Logic used for the bonus task in step 1.1. */
module Bonus {
  /**
   * Holds if `isValidMethod` is declared on the constraint validator `validatorType`
   * and is used to validate the field/property `validatedField`,
   * whose value may be obtained from the user-controlled `remoteInput`.
   */
  predicate validatesUserControlledBeanProperty(
    ConstraintValidatorIsValidMethod isValidMethod, Field validatedField, RefType validatorType,
    RemoteFlowSource remoteInput
  ) {
    // This `isValid` method is used to validate a field, or the field's class.
    validatorType = isValidMethod.getDeclaringType() and
    (
      validatedConstraint(validatedField, _, _, validatorType) or
      validatedConstraint(validatedField.getDeclaringType(), _, _, validatorType)
    ) and
    // The value of the field is obtained from user input.
    any(UserInputToValidatedFieldConfig config)
        .hasFlow(remoteInput, DataFlow::exprNode(validatedField.getAnAssignedValue()))
  }

  /** The interface `javax.validation.Constraint`. */
  class TypeConstraint extends Interface {
    TypeConstraint() { this.hasQualifiedName("javax.validation", "Constraint") }
  }

  /** A `@Constraint` annotation. */
  class ConstraintAnnotation extends Annotation {
    ConstraintAnnotation() { this.getType() instanceof TypeConstraint }

    /** Holds if this constraint is validated by the class `validatorType`. */
    predicate isValidatedBy(RefType validatorType) {
      validatorType =
        this.getValue("validatedBy").(ArrayInit).getAnInit().(TypeLiteral).getTypeName().getType()
    }
  }

  /**
   * Holds if `validatedElement` is annotated with a validation constraint defined by `constraintType`,
   * which in turn is annotated with `constraintAnnotation` and validated by `validatorType`.
   */
  predicate validatedConstraint(
    Annotatable validatedElement, RefType constraintType, ConstraintAnnotation constraintAnnotation,
    RefType validatorType
  ) {
    validatedElement.getAnAnnotation().getType() = constraintType and
    constraintType.getAnAnnotation() = constraintAnnotation and
    constraintAnnotation.isValidatedBy(validatorType)
  }

  // This 
  import semmle.code.java.dataflow.TaintTracking2

  /**
   * Taint tracking configuration describing the flow of data
   * from user input to a field.
   * This is used in the bonus part of step 1.1, to detect user-controlled bean properties.
   *
   * This analysis will be used to compute the sources of the original taint tracking configuration,
   * so we use a second copy of the taint tracking library to avoid compiler warnings due to
   * recursion through a negation.
   */
  class UserInputToValidatedFieldConfig extends TaintTracking2::Configuration {
    UserInputToValidatedFieldConfig() { this = "UserInputToValidatedFieldConfig" }

    override predicate isSource(DataFlow2::Node source) { source instanceof RemoteFlowSource }

    override predicate isSink(DataFlow2::Node sink) {
      sink.asExpr() = any(Field field).getAnAssignedValue()
    }
  }

  /**
   * Tracks the flow of data from a protobuf message to a corresponding Java object,
   * created using a method called `toCore<Name>`.
   * This additional step is used to track from remote input parameters
   * to bean validation of their values.
   */
  class ProtoToCoreTaintStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
      exists(MethodAccess ma |
        ma.getMethod().getName().matches("toCore%") and
        n2.asExpr() = ma and
        n1.asExpr() = ma.getArgument(0)
      )
    }
  }

  /**
   * Tracks the flow of data through utility methods in the class `CollectionsExt`.
   * This additional step is used to track from remote input parameters
   * to bean validation of their values.
   */
  class CollectionsExtTaintStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
      exists(MethodAccess ma |
        ma.getMethod().getName() = ["nullableImmutableCopyOf", "nonNull"] and
        ma.getMethod().getDeclaringType().getName() = "CollectionsExt" and
        n2.asExpr() = ma and
        n1.asExpr() = ma.getArgument(0)
      )
    }
  }
}

/* Step 1.2 */
/** The type `javax.validation.ConstraintValidatorContext`. */
class TypeConstraintValidatorContext extends RefType {
  TypeConstraintValidatorContext() {
    this.hasQualifiedName("javax.validation", "ConstraintValidatorContext")
  }
}

/**
 * A method named `buildConstraintViolationWithTemplate` declared on a subtype
 * of `javax.validation.ConstraintValidatorContext`.
 */
class BuildConstraintViolationWithTemplateMethod extends Method {
  BuildConstraintViolationWithTemplateMethod() {
    this.getDeclaringType().getASupertype*() instanceof TypeConstraintValidatorContext and
    this.hasName("buildConstraintViolationWithTemplate")
  }
}

/* Step 1.3 */
class TaintConfig extends TaintTracking::Configuration {
  TaintConfig() { this = "TaintConfig" }

  /* Step 1.1 */
  override predicate isSource(DataFlow::Node source) { source instanceof BeanValidationSource }

  /* Step 1.2 */
  override predicate isSink(DataFlow::Node sink) {
    exists(MethodAccess ma |
      ma.getMethod() instanceof BuildConstraintViolationWithTemplateMethod and
      sink.asExpr() = ma.getArgument(0)
    )
  }

  /* step 2 */
  override int explorationLimit() { result = 4 }
}

/* Step 1.5, 1.6 */
/** Tracks the flow of taint through getter calls: `x -> x.get*(...)`. */
class GetterTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(MethodAccess ma |
      (
        // Two alternatives for identifying getters.
        ma.getMethod() instanceof GetterMethod or
        ma.getMethod().getName().matches("get%")
      ) and
      n1.asExpr() = ma.getQualifier() and
      n2.asExpr() = ma
    )
  }
}

/* Step 1.6 */
import semmle.code.java.Maps

/** A call to the method `keySet()` declared on a subtype of `java.util.Map`. */
class MapKeySetCall extends MethodAccess {
  MapKeySetCall() {
    this.getMethod().(MapMethod).getName() = "keySet"
    // Alternative, without using the `Maps` library:
    // this.getMethod().getDeclaringType().getSourceDeclaration().hasQualifiedName("java.util", "Map")
  }
}

/** Tracks the flow of taint from `x` to `x.keySet()`. */
class KeySetTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(MapKeySetCall call |
      n1.asExpr() = call.getQualifier() and
      n2.asExpr() = call
    )
  }
}

/* Step 1.7 */
/** A call to the constructor `java.util.HashSet<>(...)`. */
class HashSetConstructorCall extends Call {
  HashSetConstructorCall() {
    this
        .(ConstructorCall)
        .getConstructedType()
        .getSourceDeclaration()
        .hasQualifiedName("java.util", "HashSet")
  }
}

/** Tracks the flow of taint from `x` to `new HashSet(x)`. */
class HashSetTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(HashSetConstructorCall call |
      n1.asExpr() = call.getAnArgument() and
      n2.asExpr() = call
    )
  }
}

/* Step 1.8: no longer necessary in recent versions of the libraries */
import semmle.code.java.Collections

/** A call to the method `retainAll` declared in a collection type. */
class CollectionRetainAllCall extends MethodAccess {
  CollectionRetainAllCall() {
    this.getMethod().(CollectionMethod).getName() = "retainAll"
    // Alternative, without using the `Collections` library:
    // this.getMethod().getDeclaringType().getASourceSupertype+().hasQualifiedName("java.util", "Collection")
  }
}

/**
 * Tracks the flow of taint from `x` to `y.retainAll(x)`,
 * where `x` and `y` are both collections.
 */
class CollectionRetainAllTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(CollectionRetainAllCall ma |
      n1.asExpr() = ma.getAnArgument() and
      n2.asExpr() = ma.getQualifier()
    )
  }
}

/* Step 2 */
/** A call to the method `stream` declared in a collection type. */
class CollectionStreamCall extends MethodAccess {
  CollectionStreamCall() { this.getMethod().(CollectionMethod).getName() = "stream" }
}

/** Track taint from `x` to `x.stream()` where `x` is a collection. */
class CollectionStreamTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(CollectionStreamCall call |
      n1.asExpr() = call.getQualifier() and
      n2.asExpr() = call
    )
  }
}

/** The interface `java.util.stream.Stream`. */
class TypeStream extends Interface {
  TypeStream() { this.hasQualifiedName("java.util.stream", "Stream") }
}

/** A method declared in a stream type, that is, a subtype of `java.util.stream.Stream`. */
class StreamMethod extends Method {
  StreamMethod() { this.getDeclaringType().getASourceSupertype+() instanceof TypeStream }
}

/** A call to the method `map` declared in a stream type. */
class StreamMapCall extends MethodAccess {
  StreamMapCall() { this.getMethod().(StreamMethod).getName() = "map" }
}

/** Track taint from `stream` to `stream.map(lambda)`. */
class StreamMapTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(StreamMapCall call |
      n1.asExpr() = call.getQualifier() and
      n2.asExpr() = call
    )
  }
}

/** A call to the method `collect` declared in a stream type. */
class StreamCollectCall extends MethodAccess {
  StreamCollectCall() { this.getMethod().(StreamMethod).getName() = "collect" }
}

/** Track taint from `stream` to `stream.collect()`. */
class StreamCollectTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(StreamCollectCall call |
      n1.asExpr() = call.getQualifier() and
      n2.asExpr() = call
    )
  }
}

/* Step 3 */
/**
 * Track the flow of taint from the argument of a call to an exception message that may be thrown by that call.
 * For example, from `tainted` to `e.getMessage()` in:
 * ```java
 * try {
 *   call(tainted);
 * } catch(Exception e) {
 *   ... e.getMessage()
 * }
 * ```
 */
class ExceptionTaintStep extends TaintTracking::AdditionalTaintStep {
  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
    exists(Call call, TryStmt try, CatchClause catch, MethodAccess getMessageCall |
      // the call is within the `try` block, which has a corresponding `catch` clause
      call.getEnclosingStmt().getEnclosingStmt*() = try.getBlock() and
      try.getACatchClause() = catch and
      // the `catch` clause is likely to catch an exception thrown by the call
      (
        catch.getACaughtType().getASupertype*() = call.getCallee().getAThrownExceptionType() or
        catch.getACaughtType().getASupertype*() instanceof TypeRuntimeException
      ) and
      // the exception message is read by `getMessageCall` within the `catch` block
      catch.getVariable().getAnAccess() = getMessageCall.getQualifier() and
      getMessageCall.getMethod().getName().regexpMatch("get(Localized)?Message|toString") and
      // taint flows from any argument of the call to a place where the exception message is accessed
      n1.asExpr() = call.getAnArgument() and
      n2.asExpr() = getMessageCall
    )
  }
}

/* Step 1.3, 1.8, 2, 3 */
from TaintConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"
/* Step 1.4 */
// from TaintConfig cfg, DataFlow::PartialPathNode source, DataFlow::PartialPathNode sink
// where
//   cfg.hasPartialFlow(source, sink, _) and
//   source.getNode().getLocation().getFile().getBaseName() = "SchedulingConstraintValidator.java"
// select sink, source, sink, "Partial flow from unsanitized user data"