Help with query - unable to find path #867

akanksha1331 · 2024-11-29T04:21:23Z

akanksha1331
Nov 29, 2024

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?

Answered by redsun82

Nov 29, 2024

👋 @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(

View full answer

redsun82 · 2024-11-29T09:51:03Z

redsun82
Nov 29, 2024
Collaborator

👋 @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?

1 reply

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Uh oh!

Help with query - unable to find path #867

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Replies: 1 comment 1 reply

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Help with query - unable to find path #867

Uh oh!

Uh oh!

akanksha1331 Nov 29, 2024

Replies: 1 comment · 1 reply

Uh oh!

redsun82 Nov 29, 2024 Collaborator

Uh oh!

akanksha1331 Nov 30, 2024 Author

akanksha1331
Nov 29, 2024

Replies: 1 comment 1 reply

redsun82
Nov 29, 2024
Collaborator

akanksha1331 Nov 30, 2024
Author