Skip to content

Commit

Permalink
Update exploitability rules handle to exclude the root callable name
Browse files Browse the repository at this point in the history
Summary:
As discussed during program analysis week, this diff removes the root "callable" from the issue handle for exploitability rules.

The handle will now only consist of the exploitability_origin where:
- `explotability_origin.exploitability_root` == the original source to sink flow root method.
- `explotability_origin.callee` == the callee (method or field sink) of the original source to sink flow root method.

Note to self:
- Previous issue with duplicate master issue handles in the *same* root callable: T201904683

Reviewed By: arthaud

Differential Revision: D66032691

fbshipit-source-id: fe102ac7668862c58021f1d739cf62955bb60b31
  • Loading branch information
Anwesh Tuladhar authored and facebook-github-bot committed Nov 25, 2024
1 parent fdcbd5e commit dc4a5b6
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
3 changes: 1 addition & 2 deletions sapp/pipeline/mariana_trench_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,7 @@ def get_master_handle(
issue_line: int,
) -> str:
return BaseParser.compute_handle_from_key(
f"{callable}"
f":{issue_callee.to_sapp_handle(callable_line, issue_line)}"
f"{issue_callee.to_sapp_handle(callable, callable_line, issue_line)}"
f":{sink_index}"
f":{code}"
)
Expand Down
10 changes: 8 additions & 2 deletions sapp/pipeline/mariana_trench_parser_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,13 +461,19 @@ def _strip_anonymous_class_numbers(
f"{stripped_classname}#{relative_line}{callee_signature[first_semicolon:]}"
)

def to_sapp_handle(self, callable_line: int, issue_line: int) -> str:
def to_sapp_handle(self, callable: str, callable_line: int, issue_line: int) -> str:
if isinstance(self.callee, ExploitabilityOrigin):
# We do not include `callable` in the handle for exploitability issues.
# This way, SAPP UI groups all issues with the same exploitability origin
# together in the unique issues view.
# This does mean that we will write multiple issue instances with the
# same issue handle.
return f"{self.callee.exploitability_root}:{self.callee.callee}"
elif isinstance(self.callee, str):
return IssueCallee._strip_anonymous_class_numbers(
callee = IssueCallee._strip_anonymous_class_numbers(
self.callee, callable_line, issue_line
)
return f"{callable}:{callee}"
else:
raise AssertionError(
"Unreachable state for IssueCallee.get_handle() method"
Expand Down

0 comments on commit dc4a5b6

Please sign in to comment.