|
| 1 | +/** |
| 2 | + * @name Using 'toString' in query logic |
| 3 | + * @description A query should not depend on the output of 'toString'. |
| 4 | + * @kind problem |
| 5 | + * @problem.severity error |
| 6 | + * @id ql/to-string-in-logic |
| 7 | + * @precision medium |
| 8 | + * @tags maintainability |
| 9 | + */ |
| 10 | + |
| 11 | +import ql |
| 12 | +import codeql_ql.ast.internal.Builtins |
| 13 | +import codeql.dataflow.DataFlow |
| 14 | + |
| 15 | +class ToString extends Predicate { |
| 16 | + ToString() { this.getName() = "toString" } |
| 17 | +} |
| 18 | + |
| 19 | +class ToStringCall extends Call { |
| 20 | + ToStringCall() { this.getTarget() instanceof ToString } |
| 21 | +} |
| 22 | + |
| 23 | +class NodesPredicate extends Predicate { |
| 24 | + NodesPredicate() { this.getName() = "nodes" } |
| 25 | +} |
| 26 | + |
| 27 | +class EdgesPredicate extends Predicate { |
| 28 | + EdgesPredicate() { this.getName() = "edges" } |
| 29 | +} |
| 30 | + |
| 31 | +class RegexpReplaceAll extends BuiltinPredicate { |
| 32 | + RegexpReplaceAll() { this.getName() = "regexpReplaceAll" } |
| 33 | +} |
| 34 | + |
| 35 | +class RegexpReplaceAllCall extends MemberCall { |
| 36 | + RegexpReplaceAllCall() { this.getTarget() instanceof RegexpReplaceAll } |
| 37 | +} |
| 38 | + |
| 39 | +class ToSelectConf extends DataFlow::Configuration { |
| 40 | + ToSelectConf() { this = "ToSelectConf" } |
| 41 | + |
| 42 | + override predicate isSource(DataFlow::Node source) { |
| 43 | + exists(ToStringCall toString | |
| 44 | + source.asExpr() = toString and |
| 45 | + not toString.getEnclosingPredicate() instanceof ToString |
| 46 | + ) |
| 47 | + } |
| 48 | + |
| 49 | + override predicate isSink(DataFlow::Node sink) { |
| 50 | + sink.asExpr() = any(Select s).getExpr(_) or |
| 51 | + sink.getEnclosingCallable().asPredicate() instanceof NodesPredicate or |
| 52 | + sink.getEnclosingCallable().asPredicate() instanceof EdgesPredicate |
| 53 | + } |
| 54 | + |
| 55 | + override predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { |
| 56 | + nodeFrom.getType() instanceof StringClass and |
| 57 | + nodeTo.getType() instanceof StringClass and |
| 58 | + exists(BinOpExpr binop | |
| 59 | + nodeFrom.asExpr() = binop.getAnOperand() and |
| 60 | + nodeTo.asExpr() = binop |
| 61 | + ) |
| 62 | + or |
| 63 | + nodeTo.asExpr().(RegexpReplaceAllCall).getBase() = nodeFrom.asExpr() |
| 64 | + } |
| 65 | +} |
| 66 | + |
| 67 | +predicate flowsToSelect(Expr e) { |
| 68 | + exists(DataFlow::Node source | |
| 69 | + source.asExpr() = e and |
| 70 | + any(ToSelectConf conf).hasFlow(source, _) |
| 71 | + ) |
| 72 | +} |
| 73 | + |
| 74 | +from ToStringCall call |
| 75 | +where |
| 76 | + // The call doesn't flow to a select |
| 77 | + not flowsToSelect(call) and |
| 78 | + // It's not part of a toString call |
| 79 | + not call.getEnclosingPredicate() instanceof ToString and |
| 80 | + // It's in a query |
| 81 | + call.getLocation().getFile().getBaseName().matches("%.ql") and |
| 82 | + // ... and not in a test |
| 83 | + not call.getLocation() |
| 84 | + .getFile() |
| 85 | + .getAbsolutePath() |
| 86 | + .toLowerCase() |
| 87 | + .matches(["%test%", "%consistency%", "%meta%"]) |
| 88 | +select call, "Query logic depends on implementation of 'toString'." |
0 commit comments