php taint analysis has duplicate results

[prabhu]

Sometimes the same result is reported 3 or 4 times. Possibly a downstream defect in the tool but needs investigation.

Example

{
          "message": {
            "markdown": "",
            "text": "Detected tainted shell."
          },
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "region": {
                  "snippet": {
                    "text": "\t\tif (false === file_put_contents($this->path, $file, LOCK_EX)) {\n"
                  },
                  "startLine": 246
                },
                "artifactLocation": {
                  "uri": "app/ConfigFile.php"
                },
                "contextRegion": {
                  "snippet": {
                    "text": "\t\t}\n\t\tif (false === file_put_contents($this->path, $file, LOCK_EX)) {\n\t\t\tthrow new Exceptions\\AppException(\"ERR_CREATE_FILE_FAILURE||{$this->path}\");\n\t\t}\n"
                  },
                  "endLine": 248,
                  "startLine": 245
                }
              }
            }
          ],
          "properties": {
            "issue_confidence": "MEDIUM",
            "issue_severity": "CRITICAL"
          },
          "baselineState": "new",
          "partialFingerprints": {},
          "ruleId": "TaintedInput",
          "ruleIndex": 0
        },
        {
          "message": {
            "markdown": "",
            "text": "Detected tainted shell."
          },
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "region": {
                  "snippet": {
                    "text": "\t\tif (false === file_put_contents($this->path, $file, LOCK_EX)) {\n"
                  },
                  "startLine": 246
                },
                "artifactLocation": {
                  "uri": "app/ConfigFile.php"
                },
                "contextRegion": {
                  "snippet": {
                    "text": "\t\t}\n\t\tif (false === file_put_contents($this->path, $file, LOCK_EX)) {\n\t\t\tthrow new Exceptions\\AppException(\"ERR_CREATE_FILE_FAILURE||{$this->path}\");\n\t\t}\n"
                  },
                  "endLine": 248,
                  "startLine": 245
                }
              }
            }
          ],
          "properties": {
            "issue_confidence": "MEDIUM",
            "issue_severity": "CRITICAL"
          },
          "baselineState": "new",
          "partialFingerprints": {},
          "ruleId": "TaintedInput",
          "ruleIndex": 0
        },
        {
          "message": {
            "markdown": "",
            "text": "Detected tainted shell."
          },
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "region": {
                  "snippet": {
                    "text": "\t\tif (false === file_put_contents($this->path, $file, LOCK_EX)) {\n"
                  },
                  "startLine": 246
                },
                "artifactLocation": {
                  "uri": "app/ConfigFile.php"
                },
                "contextRegion": {
                  "snippet": {
                    "text": "\t\t}\n\t\tif (false === file_put_contents($this->path, $file, LOCK_EX)) {\n\t\t\tthrow new Exceptions\\AppException(\"ERR_CREATE_FILE_FAILURE||{$this->path}\");\n\t\t}\n"
                  },
                  "endLine": 248,
                  "startLine": 245
                }
              }
            }
          ],
          "properties": {
            "issue_confidence": "MEDIUM",
            "issue_severity": "CRITICAL"
          },
          "baselineState": "new",
          "partialFingerprints": {},
          "ruleId": "TaintedInput",
          "ruleIndex": 0
        },

[prabhu]

Tracked here vimeo/psalm#3773

[muglug]

I think this tool needs to provide some information about the different paths, otherwise users might think these are duplicate results.

[prabhu]

@muglug
Thanks for the clarification. I will try to understand the new json format and see if the source and sink can be represented better.

[prabhu]

This is implemented by highlighting the source for now. There are still many results being returned and it is not clear whether all of them are valid. Have to filter out false positives at some point.