René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"isSanitizerGuard works incorrectly when the function name startwith \"isValid\"","articleBody":"i am try the the codeql query from https://codeql.github.com/docs/codeql-language-guides/analyzing-data-flow-in-javascript-and-typescript/ ; \r\nthe code to analysis is copied from tutorial\r\n```\r\nconst fs = require('fs'),\r\n      path = require('path');\r\n\r\nfunction readFileHelper(p) {     // #2\r\n  if (!checkPath(p)){\r\n    return;\r\n  }\r\n  fs.readFile(p,                 // #4\r\n    'utf8', (err, data) =\u003e {\r\n    if (err) throw err;\r\n    console.log(data);\r\n  });\r\n}\r\n\r\nreadFileHelper(process.argv[2]); // #1\r\n```\r\nand the query is also from tutorial\r\n```\r\n/**\r\n * @name Find usage of request.body and request.query in Express.js\r\n * @description Finds variables assigned from request.body and request.query in Express.js applications.\r\n * @kind problem\r\n * @id js/express-request-body-query\r\n */\r\nimport javascript\r\n\r\nclass CheckPathSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {\r\n  CheckPathSanitizerGuard() { this.getCalleeName() = \"checkPath\" }\r\n\r\n  override predicate sanitizes(boolean outcome, Expr e) {\r\n    outcome = true and\r\n    e = getArgument(0).asExpr()\r\n  }\r\n}\r\n\r\nclass CommandLineFileNameConfiguration extends TaintTracking::Configuration {\r\n  CommandLineFileNameConfiguration() { this = \"CommandLineFileNameConfiguration\" }\r\n\r\n  override predicate isSource(DataFlow::Node source) {\r\n    DataFlow::globalVarRef(\"process\").getAPropertyRead(\"argv\").getAPropertyRead() = source\r\n  }\r\n\r\n  override predicate isSink(DataFlow::Node sink) {\r\n    DataFlow::moduleMember(\"fs\", \"readFile\").getACall().getArgument(0) = sink\r\n  }\r\n  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {\r\n    nd instanceof CheckPathSanitizerGuard\r\n  }\r\n}\r\n\r\nfrom CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink\r\nwhere cfg.hasFlow(source, sink)\r\nselect source, \"12312323\" \r\n```\r\ni use command line cli (2.18.3 newest version) to get the result\r\n```\r\ncodeql database create codeqldb --language=javascript\r\ncodeql database analyze --rerun --threads 0 \u003cdir of ql\u003e/test2.ql --format=sarif-latest --output=1.sarif\r\n```\r\nIt all works fine.\r\nThen i change the ql, this line \r\n```\r\n  override predicate sanitizes(boolean outcome, Expr e) {\r\n    outcome = true and\r\n    e = getArgument(0).asExpr()\r\n  }\r\n``` \r\nto\r\n```\r\n  override predicate sanitizes(boolean outcome, Expr e) {\r\n    outcome = false and\r\n    e = getArgument(0).asExpr()\r\n  }\r\n```\r\nAs expected, it does not sanitize the output. \r\nThe wierd thing is, if i change the function name of \"checkPath\", to \"isValid\", the sanitizer works very wierd, it sanitize the output no matter outcome is true of false. code is like:\r\n```\r\nconst fs = require('fs'),\r\n      path = require('path');\r\n\r\nfunction readFileHelper(p) {     // #2\r\n  if (!isValid(p)){\r\n    return;\r\n  }\r\n  fs.readFile(p,                 // #4\r\n    'utf8', (err, data) =\u003e {\r\n    if (err) throw err;\r\n    console.log(data);\r\n  });\r\n}\r\n\r\nreadFileHelper(process.argv[2]); // #1\r\n```\r\nql is like\r\n```\r\n/**\r\n * @name Find usage of request.body and request.query in Express.js\r\n * @description Finds variables assigned from request.body and request.query in Express.js applications.\r\n * @kind problem\r\n * @id js/express-request-body-query\r\n */\r\nimport javascript\r\n\r\nclass CheckPathSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {\r\n  CheckPathSanitizerGuard() { this.getCalleeName() = \"isValid\" }\r\n\r\n  override predicate sanitizes(boolean outcome, Expr e) {\r\n    outcome = false and\r\n    e = getArgument(0).asExpr()\r\n  }\r\n}\r\n\r\nclass CommandLineFileNameConfiguration extends TaintTracking::Configuration {\r\n  CommandLineFileNameConfiguration() { this = \"CommandLineFileNameConfiguration\" }\r\n\r\n  override predicate isSource(DataFlow::Node source) {\r\n    DataFlow::globalVarRef(\"process\").getAPropertyRead(\"argv\").getAPropertyRead() = source\r\n  }\r\n\r\n  override predicate isSink(DataFlow::Node sink) {\r\n    DataFlow::moduleMember(\"fs\", \"readFile\").getACall().getArgument(0) = sink\r\n  }\r\n  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {\r\n    nd instanceof CheckPathSanitizerGuard\r\n  }\r\n}\r\n\r\nfrom CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink\r\nwhere cfg.hasFlow(source, sink)\r\nselect source, \"12312323\" \r\n```\r\n\r\nThe only thing changed is the function name. \"isValid\" is not some special thing in js i think. I try other function names, everything start with isValid works abnormal, like \"isValidd\" \"isValidg\", anything else works correctly, like \"isVali\"","author":{"url":"https://github.com/oicu0619","@type":"Person","name":"oicu0619"},"datePublished":"2024-09-06T02:05:13.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":1},"url":"https://github.com/17393/codeql/issues/17393"}