Missing Flows in Backward Slicing.

[KylerKatz]

Hello, I am using CodeQL to perform backward slicing. However, I am noticing that my query is currently missing some flows.

I have this example,

import javax.mail.*;
import javax.mail.internet.*;
import java.util.Properties;

public class BAD_EmailHeaderExposure {
    public void sendEmailWithSensitiveHeader(String recipient, String sessionToken) {
        Properties properties = System.getProperties();
        properties.setProperty("mail.smtp.host", "smtp.internal.books.com");
        Session session = Session.getDefaultInstance(properties, null);

        try {
            MimeMessage message = new MimeMessage(session);
            message.setRecipient(Message.RecipientType.TO, new InternetAddress(recipient));
            message.setSubject("Here is your session Info");
            message.addHeader("X-Session-Token", sessionToken);
            Transport.send(message);
        } catch (MessagingException e) {
            System.out.println("Failed to send email: " + e.getMessage());
        }
    }
}

and I am using this query

/**
 * @name Backward slicing
 * @description Identifies the backward slice of a sink node
 * @kind path-problem
 * @problem.severity warning
 * @id java/backward-slice
 */

 import java
 private import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.java.dataflow.TaintTracking
 
 module Flow = TaintTracking::Global<AllVariablesConfig>;
 import Flow::PathGraph
 
 /** A configuration for tracking data flow between all variables. */
 module AllVariablesConfig implements DataFlow::ConfigSig {
 
   // Any variable can be a source
   predicate isSource(DataFlow::Node source) {
     source.asExpr() instanceof Literal
     or
     exists(Variable v |
       source.asExpr() = v.getAnAccess()
     )
     or
     source.asExpr() instanceof MethodCall
     or
     source.asExpr() instanceof NewClassExpr
     or
     source.asExpr() instanceof FieldAccess
     or
     exists(Parameter p |
       source.asExpr() = p.getAnAccess()
     )
   }
 
   // Any variable can be a sink
   predicate isSink(DataFlow::Node sink) {
     exists(Variable v |
       sink.asExpr() = v.getAnAccess()
     )
   }
 
   // Do not skip any nodes
   predicate neverSkip(DataFlow::Node node) {
     any()
   }
 
 }
 
 from Flow::PathNode source, Flow::PathNode sink
 where Flow::flowPath(source, sink) and
     // Exclude cases where the source and sink are the same node
     source.getNode().toString() != sink.getNode().toString()
 select sink.getNode(), source, sink, 
    "Dataflow from `" + source.getNode().toString() + "` to `" + sink.getNode().toString() + "`"

Now if you take session for example,

There is no direct flow from properties to session. The closest I get is

            {
                "name": "session",
                "graph": [
                    {
                        "name": "Session.getDefaultInstance(properties, null)",
                        "type": "Session",
                        "context": "        Properties properties = System.getProperties();\r\n        properties.setProperty(\"mail.smtp.host\", \"smtp.internal.books.com\");\r\n        Session session = Session.getDefaultInstance(properties, null);\r\n\r\n        try {\r\n",
                        "nextNode": "session"
                    },
                    {
                        "name": "session",
                        "type": "Dataflow from `getDefaultInstance(...)` to `session`",
                        "context": "\r\n        try {\r\n            MimeMessage message = new MimeMessage(session);\r\n            message.setRecipient(Message.RecipientType.TO, new InternetAddress(recipient));\r\n            message.setSubject(\"Here is your session Info\");\r\n",
                        "nextNode": "end"
                    }
                ]
            },

We can see that properties gets passed to Session.getDefaultInstance() however, I would like it to also show where properties comes from.

The source in my query has expanded a bit in trying to get this to work, however, why is it not as simple as a variable for the source and a variable for the sink? I would appreciate any help with this. Thank you.

[rvermeulen]

Hi @KylerKatz, thanks for your question.

I did a quick test on the snippet below using your query and my first result was the path System.getProperties() -> properties -> Session.getDefaultInstance(properties,...)

If I understand correctly you want to have the path all the way to a use of session.

If that is the case then you need to inform the configuration you want the result of a method call to be considered tainted when one of its parameters is. This is not the default because it is not always true.

import java.util.Properties;

// Stub for javax.mail.Session
class Session {
    public static Session getDefaultInstance(Properties props, Object auth) {
        return new Session();
    }
}

// Stub for javax.mail.internet.MimeMessage
class MimeMessage {
    public MimeMessage(Session session) {
        // Stub constructor
    }

    public void setRecipient(Message.RecipientType type, InternetAddress address) {
        // Stub method
    }

    public void setSubject(String subject) {
        // Stub method
    }

    public void addHeader(String name, String value) {
        // Stub method
    }
}

// Stub for javax.mail.Message
class Message {
    public static class RecipientType {
        public static final RecipientType TO = new RecipientType();
    }
}

// Stub for javax.mail.internet.InternetAddress
class InternetAddress {
    public InternetAddress(String address) {
        // Stub constructor
    }
}

// Stub for javax.mail.Transport
class Transport {
    public static void send(MimeMessage message) throws MessagingException {
        // Stub method
    }
}

// Stub for javax.mail.MessagingException
class MessagingException extends Exception {
    public MessagingException(String message) {
        super(message);
    }
}

class BAD_EmailHeaderExposure {
    public void sendEmailWithSensitiveHeader(String recipient, String sessionToken) {
        Properties properties = System.getProperties();
        properties.setProperty("mail.smtp.host", "smtp.internal.books.com");
        Session session = Session.getDefaultInstance(properties, null);

        try {
            MimeMessage message = new MimeMessage(session);
            message.setRecipient(Message.RecipientType.TO, new InternetAddress(recipient));
            message.setSubject("Here is your session Info");
            message.addHeader("X-Session-Token", sessionToken);
            Transport.send(message);
        } catch (MessagingException e) {
            System.out.println("Failed to send email: " + e.getMessage());
        }
    }
}

public class Test {

}

The following additional step can help with that:

   predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
     exists(MethodCall mc | succ.asExpr() = mc and mc.getAnArgument() = pred.asExpr())
   }

Let me know if this helps.

[github-actions[bot]]

This issue is stale because it has been open 14 days with no activity. Comment or remove the Stale label in order to avoid having this issue closed in 7 days.

[github-actions[bot]]

This issue was closed because it has been inactive for 7 days.