Skip to content

Commit

Permalink
Added precision, improved SARIF output
Browse files Browse the repository at this point in the history
  • Loading branch information
forefy committed Feb 16, 2024
1 parent 68cb5a8 commit 07fc469
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 15 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ dist/
**/cache
.vscode
*[0-9][0-9].py
*eburger-output*
*eburger-output*
*.sarif
2 changes: 1 addition & 1 deletion eburger/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def main():
if len(project_paths) < 1:
log(
"info",
"No contract projects here, specifiy a path/single file via `-f` or run `eburger` in the smart contract project root.",
"No contract projects here, specifiy a path/single files via `-f` or run `eburger` in the smart contract project root.",
)
sys.exit(0)
elif len(project_paths) > 1:
Expand Down
1 change: 1 addition & 0 deletions eburger/templates/missing_reentrancy_guard.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: "Missing Reentracy Guard"
severity: "Low"
precision: "Low"
description: "Due to Solidity's design, when a function in your contract interacts with another contract's address, that contract gains the ability to recursively call your contract's function before handing back the control on the execution flow. If the reentrant function's logic did not expect the function to be called multiple times, it can be exposed to abuse."
impact: "Potential draining of funds, depending on what happens when the function is retriggered."
action-items:
Expand Down
1 change: 1 addition & 0 deletions eburger/templates/missing_zero_address_check.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: "Missing Zero Address Check"
severity: "Low"
precision: "Medium"
description: "In Solidity, contracts often interact with external addresses. Failing to check for a possible 0 address input (especially in constructors, setters, and initializer functions) before such interactions can lead to unexpected dangerous behavior. A zero address check ensures that addresses are explicitly provided and not left uninitialized or set to a default, invalid state."
impact: "Unexpected failed transactions and contract behavior."
action-items:
Expand Down
3 changes: 2 additions & 1 deletion eburger/templates/modifier_without_proper_enforcement.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
name: "Modifier Without Proper Enforcement"
severity: "High"
precision: "Low"
description: "Using the provided modifier onlyOwner for function access control without a proper enforcement mechanism like require or revert is a dire mistake because it fails to restrict access as intended. The modifier merely evaluates a condition (msg.sender == owner) without any action taken based on the result."
impact: "This means any user, regardless of whether they are the owner, can execute functions that are supposed to be restricted to the owner, potentially leading to unauthorized actions, such as withdrawing funds or altering critical contract settings."
impact: "Users may execute functions that are supposed to be restricted, potentially leading to unauthorized actions, such as withdrawing funds or altering critical contract settings."
action-items:
- "Change the modifier so that it will enforce the ownership check using a require/revert statement."
references:
Expand Down
1 change: 1 addition & 0 deletions eburger/templates/tx_origin_used_for_access_control.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: "tx.origin Used for Access Control"
severity: "Low"
precision: "Medium"
description: "In a situation in which a user has interacted with a malicious contract, that contract can then impersonate as the victim user to perform actions in the current contract that are relying on the tx.origin."
impact: "Malicious contracts can impersonate users to perform actions relying on the tx.origin."
action-items:
Expand Down
3 changes: 2 additions & 1 deletion eburger/templates/unbounded_loop.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: "Unbound Loop"
name: "Unbounded Loop"
severity: "Low"
precision: "Low"
description: "Unbounded loops, specifically, for loops that can modify state and have no apparent max restriction to the nunmber of iterations possible, can lead to excessive gas consumption, which may cause transactions to fail or become prohibitively expensive."
impact: "Excessive gas consumption, contract denial of service."
action-items:
Expand Down
1 change: 1 addition & 0 deletions eburger/templates/unchecked_call_return.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: "Unchecked Call Return"
severity: "Low"
precision: "Low"
description: "A contract's call or send functions are used without checking the return value."
impact: "Unexpected contract behavior."
action-items:
Expand Down
1 change: 1 addition & 0 deletions eburger/templates/use_of_transfer_or_send_on_payable.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: "Use of transfer or send on a payable address"
severity: "Medium"
precision: "Medium"
description: "In Solidity, .transfer and .send both implement a risky gas limitation that reverts the transaction if the recipient's operations require more gas than the stipend of 2300 gas."
impact: "Unexpected failed transactions and contract behavior."
action-items:
Expand Down
41 changes: 30 additions & 11 deletions eburger/utils/outputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,25 @@ def save_as_markdown(output_file_path: Path, json_data: dict):


def convert_severity_to_sarif_level(severity: str) -> str:
match severity:
case "High":
sarif_level = "error"
case "Medium":
sarif_level = "warning"
case "Low":
sarif_level = "note"
case _:
sarif_level = "note"
return sarif_level
mapping = {"High": "error", "Medium": "warning", "Low": "note"}
mapped_severity = mapping.get(severity, None)
if mapped_severity is None:
log(
"error",
"Failed converting a template's severity to the respective SARIF level",
)
return mapped_severity


def convert_severity_to_sarif_security_severity(severity: str) -> str:
mapping = mapping = {"High": 8.0, "Medium": 5.0, "Low": 3.0}
mapped_severity = mapping.get(severity, None)
if mapped_severity is None:
log(
"error",
"Failed converting a template's severity to the respective SARIF/GitHub Code Scanning security severity",
)
return mapped_severity


def save_as_sarif(output_file_path: Path, insights: dict):
Expand All @@ -129,15 +138,20 @@ def save_as_sarif(output_file_path: Path, insights: dict):
"results": [],
}

# precision - very-high, high, medium, or low
# security-severity - 9.0 is critical, 7.0 to 8.9 is high, 4.0 to 6.9 is medium and 3.9 or less is low
# simplified: 9.0, 8.0, 5.0, 3.0
sarif_rule_template = {
"id": "",
"name": "",
"shortDescription": {"text": ""},
"fullDescription": {"text": ""},
"helpUri": "",
"help": {
"text": "",
"markdown": "",
},
"properties": {"precision": "", "problem": {"security-severity": ""}},
}

if os.path.exists(output_file_path):
Expand Down Expand Up @@ -175,12 +189,17 @@ def save_as_sarif(output_file_path: Path, insights: dict):
new_rule = sarif_rule_template.copy()
new_rule["id"] = rule_id
new_rule["name"] = insight["name"]
new_rule["shortDescription"]["text"] = insight["description"]
new_rule["shortDescription"]["text"] = insight["name"]
new_rule["fullDescription"]["text"] = insight["description"]
new_rule["helpUri"] = insight["references"][0]
new_rule["help"]["text"] = insight["action-items"][0]
new_rule["help"][
"markdown"
] = f"[{insight['action-items'][0]}]({insight['references'][0]})"
new_rule["properties"]["precision"] = insight["precision"].casefold()
new_rule["properties"]["problem"]["security-severity"] = (
convert_severity_to_sarif_security_severity(insight["severity"])
)
new_run["tool"]["driver"]["rules"].append(new_rule)
rule_index = len(new_run["tool"]["driver"]["rules"]) - 1

Expand Down
1 change: 1 addition & 0 deletions eburger/yaml_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def process_yaml(file_path, ast_data, src_file_list):
return {
"name": yaml_data.get("name"),
"severity": yaml_data.get("severity"),
"precision": yaml_data.get("precision"),
"description": yaml_data.get("description"),
"results": results,
"action-items": yaml_data.get("action-items"),
Expand Down

0 comments on commit 07fc469

Please sign in to comment.