Skip to content

After Change fopen-flow-from-getenv.ql the sarif results is None.How do I solve this? #19242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ysuLihua opened this issue Apr 8, 2025 · 2 comments
Labels
question Further information is requested

Comments

@ysuLihua
Copy link

ysuLihua commented Apr 8, 2025

test code:

#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <malloc.h>
#include <stdlib.h>

void bad(){
    char * filename1 = getenv("PATH");
    FILE * file1 = fopen(filename1, "r");
    printf("Path: %s", filename1);
}


int main(){
    bad();
}

fopen-flow-from-getenv.ql:

/**
 * @name member variable to open or create resrouce
 * @description Use a member variable to open or create resource. maybe unrelease and detect resource leaks.
 * @kind path-problem
 * @id cpp/member-variable-to-resource-leak
 * @problem.severity warning
 * @security-severity 7.8
 * @tags efficiency
 *       security
 *       external/cwe/cwe-404
 */

 import cpp
 import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 import semmle.code.cpp.ir.dataflow.TaintTracking
 import semmle.code.cpp.ir.IR
 import semmle.code.cpp.controlflow.IRGuards
 import semmle.code.cpp.security.FlowSources


 predicate isFlowSource(FlowSource source, string sourceType) { sourceType = source.getSourceType() }

 predicate openSink(DataFlow::Node sink) {
  exists(FunctionCall fc |
    sink.asIndirectExpr(1) = fc.getArgument(0) and
    fc.getTarget().hasGlobalName("fopen")
  )
}

 module EnvironmentToFileConfig implements DataFlow::ConfigSig {

   predicate isSource(DataFlow::Node source) { isFlowSource(source, _) }

   predicate isSink(DataFlow::Node sink) { openSink(sink)}

 }
 
 module Flow = DataFlow::Global<EnvironmentToFileConfig>;
 import Flow::PathGraph
 
 from Expr getenv, Expr fopen, Flow::PathNode source, Flow::PathNode sink
 where
   isFlowSource(source.getNode(), _) and
   openSink(sink.getNode()) and
   Flow::flowPath(source, sink)
 select sink.getNode().asExpr(), source, sink, "open file by tainted data "
 
@ysuLihua ysuLihua added the question Further information is requested label Apr 8, 2025
@smowton
Copy link
Contributor

smowton commented Apr 8, 2025

This is because the dataflow node representing the sink (the call to fopen) is not an expression node, and so getNode().asExpr() is undefined.

As the documentation for asExpr says:

  /**
   * Gets the non-conversion expression corresponding to this node, if any.
   * This predicate only has a result on nodes that represent the value of
   * evaluating the expression. For data flowing _out of_ an expression, like
   * when an argument is passed by reference, use `asDefiningArgument` instead
   * of `asExpr`.
   *
   * If this node strictly (in the sense of `asConvertedExpr`) corresponds to
   * a `Conversion`, then the result is the underlying non-`Conversion` base
   * expression.
   */

So this is a node kind where asExpr is not defined but asDefiningArgument is defined. However in the context of the first column of a path-query, the only purpose is to specify an alert location, so it's better to just select the node: replace select sink.getNode().asExpr() with select sink.getNode() and everything works as you might hope.

Some other comments while I'm here:

  1. I had to remove #include <malloc.h> from your sample code to compile it -- this is a nonstandard header; for malloc just import stdlib.h.
  2. The lines isFlowSource(source.getNode(), _) and openSink(sink.getNode()) next to Flow::flowPath(source, sink) are redundant: that there is a flow path implies that the two ends of the path are a source and a sink respectively. They can be deleted.
  3. from definitions Expr getenv, Expr fopen are unused and can be deleted.

@ysuLihua
Copy link
Author

ysuLihua commented Apr 8, 2025

Thanks for your comments!

@ysuLihua ysuLihua closed this as completed Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants