Help with query - unable to find path

[akanksha1331]

The codeql query to find path

/**
 * ...
 *
 * @kind path-problem
 * @name 117path
 * @id java/example/path-detection
 * ...
 */

import semmle.code.java.dataflow.DataFlow

module MyFlowConfiguration implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node src) {
    exists(Parameter p |
        p.getName() = "target" and
        src.asParameter() = p
      )
  }


  predicate isSink(DataFlow::Node sink) {
    exists(AddExpr addExpr, StringLiteral literal |
      sink.asExpr() = addExpr and
      addExpr.getLeftOperand() = literal and
      literal.getValue() = "redirect:"
    )
  }
}

module Flow = DataFlow::Global<MyFlowConfiguration>;

import Flow::PathGraph

from Flow::PathNode source, Flow::PathNode sink
where Flow::flowPath(source, sink)
select sink.getNode(), source, sink, ""

The code snippet in which the taint source and sink are present:

@RequestMapping(value = "/login", method = RequestMethod.POST)
public String processLogin(
        @RequestParam(value = "user", required = true) String username,
        @RequestParam(value = "password", required = true) String password,
        @RequestParam(value = "remember", required = false) String remember,
        **@RequestParam(value = "target", required = false) String target, // SOURCE**
        Model model,
        HttpServletRequest req,
        HttpServletResponse response) {
    logger.info("Entering processLogin");

    // Determine eventual redirect. Do this here in case we're already logged in
    String nextView;
    if (target != null && !target.isEmpty() && !target.equals("null")) {
        **nextView = "redirect:" + target; // SINK**
    } else {
        // default to user's feed
        nextView = Utils.redirect("feed");
    }

    Connection connect = null;
    Statement sqlStatement = null;

    try {
        // Get the Database Connection
        logger.info("Creating the Database connection");
        Class.forName("com.mysql.jdbc.Driver");
        connect = DriverManager.getConnection(Constants.create().getJdbcConnectionString());

        /* START EXAMPLE VULNERABILITY */
        // Execute the query
        logger.info("Creating the Statement");
        String sqlQuery = "select username, password, password_hint, created_at, last_login, real_name, blab_name from users where username='"
                + username + "' and password='" + md5(password) + "';";
        sqlStatement = connect.createStatement();
        logger.info("Execute the Statement");
        ResultSet result = sqlStatement.executeQuery(sqlQuery);
        /* END EXAMPLE VULNERABILITY */

        // Did we find exactly 1 user that matched?
        if (result.first()) {
            logger.info("User Found.");
            // Remember the username as a courtesy.
            Utils.setUsernameCookie(response, result.getString("username"));

            // If the user wants us to auto-login, store the user details as a cookie.
            if (remember != null) {
                User currentUser = new User(result.getString("username"), result.getString("password_hint"),
                        result.getTimestamp("created_at"), result.getTimestamp("last_login"),
                        result.getString("real_name"), result.getString("blab_name"));

                UserFactory.updateInResponse(currentUser, response);
            }

            Utils.setSessionUserName(req, response, result.getString("username"));

            // Update last login timestamp
            PreparedStatement update = connect.prepareStatement("UPDATE users SET last_login=NOW() WHERE username=?;");
            update.setString(1, result.getString("username"));
            update.execute();
        } else {
            // Login failed...
            logger.info("User Not Found");
            model.addAttribute("error", "Login failed. Please try again.");
            model.addAttribute("target", target);
            nextView = "login";
        }
    } catch (SQLException exceptSql) {
        logger.error(exceptSql);
        model.addAttribute("error", exceptSql.getMessage() + "<br/>" + displayErrorForWeb(exceptSql));
        model.addAttribute("target", target);
        nextView = "login";
    } catch (ClassNotFoundException cnfe) {
        logger.error(cnfe);
        model.addAttribute("error", cnfe.getMessage());
        model.addAttribute("target", target);

    } finally {
        try {
            if (sqlStatement != null) {
                sqlStatement.close();
            }
        } catch (SQLException exceptSql) {
            logger.error(exceptSql);
            model.addAttribute("error", exceptSql.getMessage());
            model.addAttribute("target", target);
        }
        try {
            if (connect != null) {
                connect.close();
            }
        } catch (SQLException exceptSql) {
            logger.error(exceptSql);
            model.addAttribute("error", exceptSql.getMessage());
            model.addAttribute("target", target);
        }
    }

    // Redirect to the appropriate place based on login actions above
    logger.info("Redirecting to view: " + nextView);
    return nextView;
}

Quick evaluation on the source and sink predicates shows that they work fine.
Quick evaluation on select statements gives me "Failed to run query: ERROR: The selection is not a valid quick-eval target. "
On running the entire query, I get no results.
Can anyone tell me what I can select in the select statements for quick evaluation?
Also, if there is something wrong in the codeql query?

[redsun82]

👋 @akanksha1331

Data flow doesn't go automatically from operands to + on strings, as opposed to taint tracking which instead adds that step.

I tested the query works with either one of these changes:

• either change from DataFlow to TaintTracking (import semmle.code.java.dataflow.TaintTracking and module Flow = TaintTracking::Global<MyFlowConfiguration>
• or make the sink not the AddExpr, but its operand (which seems slightly more idiomatic for data flow, as typically sinks are arguments or operands):

  predicate isSink(DataFlow::Node sink) {
    exists(AddExpr addExpr, StringLiteral literal |
      sink.asExpr() = addExpr.getRightOperand() and  // <- `isSink` changed here
      addExpr.getLeftOperand() = literal and
      literal.getValue() = "redirect:"
    )
  }

• or else manually add an additional flow step from operands to AddExpr on Strings:

  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
    exists(AddExpr addExpr |
      addExpr.getType().hasName("String") and
      node1.asExpr() = addExpr.getAnOperand() and
      node2.asExpr() = addExpr)
  }

Another hint both for debugging and reporting: when facing a query that doesn't show what you want, apart from debugging the query itself you can also simplify the code on which you run it on, which when posted makes it also easier for people helping you out. In this case for example this was all that really mattered for the query:

String foo(String target) {
    return "redirect:" + target;
}

As for how to use quick evaluation on text selections, those text selection must be valid CodeQL terms. For example, selecting sink.asExpr() = addExpr in your isSink will work, but selecting sink.asExpr() = addExpr and with the unterminated and will not. Maybe that is the problem you encountered?

[akanksha1331]

Thanks a lot!