Skip to content

Commit

Permalink
Display TaintConfig Error locations when available (#739)
Browse files Browse the repository at this point in the history
Summary:
**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Display TaintConfigurationError locations when available. The Ocaml binary now returns positions after Github PR: #734 (commit 59d2cf0). Parse and print when the location(s) are available.

Pull Request resolved: #739

Test Plan:
Invocation command in all of the below: `python3 -m pyre-check.client.pyre analyze --no-verify`

- Run pyre check with the following `faulty` taint.config: (modify `documentation/pysa_tutorial/exercise1`)
```json
{
  "sources": [
    {
      "name": "CustomUserControlled",
      "comment": "use to annotate user input"
    }
  ],

  "sinks": [
    {
      "name": "CodeExecution",
      "comment": "use to annotate execution of python code"
    }
  ],

  "features": [],

  "rules": [
    {
      "name": "Possible RCE:",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "User specified data may reach a code execution sink"
    }
    {
      "name": "test-duplicate",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "duplicate"
    }
  ]
}
```
- Before:
<img width="1137" alt="Screenshot 2023-05-31 at 2 45 34 PM" src="https://github.com/facebook/pyre-check/assets/8947010/3b2e24c4-98ab-4927-9838-f91592b504e5">

- After:
<img width="1137" alt="Screenshot 2023-05-31 at 2 36 49 PM" src="https://github.com/facebook/pyre-check/assets/8947010/43f6aaf1-5d0a-42ee-889c-e59f10e3beef">

- Run with the stock taint.config: (same results before and after)
<img width="1137" alt="Screenshot 2023-05-31 at 2 48 33 PM" src="https://github.com/facebook/pyre-check/assets/8947010/58aae8d6-0eeb-4cc5-b676-31dcfcdbe826">

- `tox -e py`

- Github Actions (pysa action was failing before this PR and is failing due to an unrelated issue - possibly outdated opam cache?)

Fixes part of MLH-Fellowship#82
Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>

Reviewed By: saputkin

Differential Revision: D47589562

Pulled By: arthaud

fbshipit-source-id: d17127a38096a1f95ade8648f14a853fd7ab0055
  • Loading branch information
abishekvashok authored and facebook-github-bot committed Jul 19, 2023
1 parent f7950b9 commit 2ba88c3
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 5 deletions.
47 changes: 42 additions & 5 deletions client/error.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,35 @@ class TaintConfigurationError:
path: Optional[Path]
description: str
code: int
start_line: Optional[int]
start_column: Optional[int]
stop_line: Optional[int]
stop_column: Optional[int]

@staticmethod
def from_json(error_json: Dict[str, Any]) -> "TaintConfigurationError":
try:
error_location = error_json["location"]
if error_location is not None:
start_line = error_location["start"]["line"]
start_column = error_location["start"]["column"]
stop_line = error_location["stop"]["line"]
stop_column = error_location["stop"]["column"]
else:
start_line = None
start_column = None
stop_line = None
stop_column = None
return TaintConfigurationError(
path=Path(error_json["path"])
if error_json["path"] is not None
else None,
description=error_json["description"],
code=error_json["code"],
start_line=start_line,
start_column=start_column,
stop_line=stop_line,
stop_column=stop_column,
)
except KeyError as key_error:
message = f"Missing field from error json: {key_error}"
Expand All @@ -190,11 +209,21 @@ def to_json(self) -> Dict[str, Any]:
"path": str(self.path) if self.path is not None else None,
"description": self.description,
"code": self.code,
"start_line": self.start_line,
"start_column": self.start_column,
"stop_line": self.stop_line,
"stop_column": self.stop_column,
}

def to_text(self) -> str:
path = click.style(str(self.path or "?"), fg="red")
return f"{path} {self.description}"
location = click.style(
f":{self.start_line}:{self.start_column}"
if (self.start_line is not None) and (self.start_column is not None)
else "",
fg="red",
)
return f"{path}{location} {self.description}"

def to_sarif(self) -> Dict[str, Any]:
return {
Expand All @@ -210,10 +239,18 @@ def to_sarif(self) -> Dict[str, Any]:
"uri": str(self.path) if self.path is not None else None,
},
"region": {
"startLine": 0,
"startColumn": 0,
"endLine": 0,
"endColumn": 1,
"startLine": self.start_line
if self.start_line is not None
else 0,
"startColumn": self.start_column
if self.start_column is not None
else 0,
"endLine": self.stop_line
if self.stop_line is not None
else 0,
"endColumn": self.stop_column
if self.stop_column is not None
else 1,
},
},
},
Expand Down
30 changes: 30 additions & 0 deletions client/tests/error_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,23 +134,53 @@ def assert_not_parsed(json: Dict[str, Any]) -> None:
"path": "test.py",
"description": "Some description",
"code": 1001,
"location": None,
},
expected=TaintConfigurationError(
path=Path("test.py"),
description="Some description",
code=1001,
start_line=None,
start_column=None,
stop_line=None,
stop_column=None,
),
)
assert_parsed(
{
"path": None,
"description": "Some description",
"code": 1001,
"location": None,
},
expected=TaintConfigurationError(
path=None,
description="Some description",
code=1001,
start_line=None,
start_column=None,
stop_line=None,
stop_column=None,
),
)
assert_parsed(
{
"path": None,
"description": "Some description",
"code": 1001,
"location": {
"start": {"line": 1, "column": 2},
"stop": {"line": 3, "column": 4},
},
},
expected=TaintConfigurationError(
path=None,
description="Some description",
code=1001,
start_line=1,
start_column=2,
stop_line=3,
stop_column=4,
),
)

Expand Down

0 comments on commit 2ba88c3

Please sign in to comment.