Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve technical details #12

Merged
merged 3 commits into from
Aug 2, 2023
Merged

Improve technical details #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25a42cb
Improve technical details
Aug 2, 2023
ed375c3
Increase patch level
Aug 2, 2023
a68be80
References
Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions agent/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
from typing import Any, Iterator

from urllib import parse
import magic
from ostorlab.agent.kb import kb
from ostorlab.agent.mixins import agent_report_vulnerability_mixin
Expand Down Expand Up @@ -42,13 +43,14 @@ def construct_technical_detail(vulnerability: dict[str, Any], path: str) -> str:
path = path or vulnerability.get("path", "N/A")
lines = vulnerability["extra"].get("lines", "").strip()
technology = vulnerability["extra"].get("metadata", {}).get("technology", [""])[0]
title = construct_vulnerability_title(check_id)

technical_detail = f"""The file `{path}` has a security issue at line `{line}`, column `{col}`:
technical_detail = f"""{title}: {message}

The issue was detected in `{path}`, line `{line}`, column `{col}`, below is a code snippet from the vulnerable code
```{technology}
{lines}
```

The issue was identified as `{check_id}` and the message from the code analysis is `{message}`."""
```"""

return technical_detail

Expand Down Expand Up @@ -88,8 +90,8 @@ def parse_results(json_output: dict[str, Any]) -> Iterator[Vulnerability]:
impact = metadata.get("impact", "UNKNOWN")
fix = extra.get("fix", "")
references = {
f"Reference: #{idx + 1}": value
for (idx, value) in enumerate(metadata.get("references", []))
parse.urlparse(value).netloc or value: value
for value in metadata.get("references", [])
}

technical_detail = construct_technical_detail(vulnerability, path)
Expand Down
2 changes: 1 addition & 1 deletion ostorlab.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
kind: Agent
name: semgrep
version: 0.2.2
version: 0.2.3
description: |
This repository is an implementation of [Ostorlab Agent](https://pypi.org/project/ostorlab/) for [Semgrep](https://github.com/returntocorp/semgrep) by r2c.
## Getting Started
Expand Down
73 changes: 42 additions & 31 deletions tests/semgrep_agent_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,19 @@ def testAgentSemgrep_whenAnalysisRunsWithoutErrors_emitsBackVulnerability(
"valid or invalid padding. Further, CBC mode does not include any "
"integrity checks. Use 'AES/GCM/NoPadding' instead."
)

assert [entry["url"] for entry in vuln["references"]] == [
"https://capec.mitre.org/data/definitions/463.html",
"https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html",
"https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY",
assert vuln["references"] == [
{
"title": "capec.mitre.org",
"url": "https://capec.mitre.org/data/definitions/463.html",
},
{
"title": "cheatsheetseries.owasp.org",
"url": "https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html",
},
{
"title": "find-sec-bugs.github.io",
"url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY",
},
]
assert not any(
[
Expand All @@ -154,19 +162,16 @@ def testAgentSemgrep_whenAnalysisRunsWithoutErrors_emitsBackVulnerability(
assert vuln["security_issue"] is True
assert (
vuln["technical_detail"]
== "The file `tests/files/vulnerable.java` has a security issue at line `28`, column "
"`44`:\n"
== "Cbc Padding Oracle: Using CBC with PKCS5Padding is susceptible to padding "
"oracle attacks. A malicious actor could discern the difference between "
"plaintext with valid or invalid padding. Further, CBC mode does not include "
"any integrity checks. Use 'AES/GCM/NoPadding' instead.\n"
" \n"
"The issue was detected in `tests/files/vulnerable.java`, line `28`, column "
"`44`, below is a code snippet from the vulnerable code\n"
"```java\n"
"Cipher cipher = Cipher.getInstance('AES/CBC/PKCS5Padding');\n"
"```\n"
"\n"
"The issue was identified as "
"`java.lang.security.audit.cbc-padding-oracle.cbc-padding-oracle` and the "
"message from the code analysis is `Using CBC with PKCS5Padding is "
"susceptible to padding oracle attacks. A malicious actor could discern the "
"difference between plaintext with valid or invalid padding. Further, CBC "
"mode does not include any integrity checks. Use 'AES/GCM/NoPadding' "
"instead.`."
"```"
)


Expand Down Expand Up @@ -200,10 +205,19 @@ def testAgentSemgrep_whenAnalysisRunsWithoutPathWithoutErrors_emitsBackVulnerabi
"integrity checks. Use 'AES/GCM/NoPadding' instead."
)

assert [entry["url"] for entry in vuln["references"]] == [
"https://capec.mitre.org/data/definitions/463.html",
"https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html",
"https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY",
assert vuln["references"] == [
{
"title": "capec.mitre.org",
"url": "https://capec.mitre.org/data/definitions/463.html",
},
{
"title": "cheatsheetseries.owasp.org",
"url": "https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html",
},
{
"title": "find-sec-bugs.github.io",
"url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY",
},
]
assert not any(
[
Expand All @@ -217,19 +231,16 @@ def testAgentSemgrep_whenAnalysisRunsWithoutPathWithoutErrors_emitsBackVulnerabi
assert vuln["security_issue"] is True
assert (
vuln["technical_detail"]
== "The file `/tmp/tmpza6g8qu0.java` has a security issue at line `28`, column "
"`44`:\n"
== "Cbc Padding Oracle: Using CBC with PKCS5Padding is susceptible to padding "
"oracle attacks. A malicious actor could discern the difference between "
"plaintext with valid or invalid padding. Further, CBC mode does not include "
"any integrity checks. Use 'AES/GCM/NoPadding' instead.\n"
" \n"
"The issue was detected in `/tmp/tmpza6g8qu0.java`, line `28`, column `44`, "
"below is a code snippet from the vulnerable code\n"
"```java\n"
"Cipher cipher = Cipher.getInstance('AES/CBC/PKCS5Padding');\n"
"```\n"
"\n"
"The issue was identified as "
"`java.lang.security.audit.cbc-padding-oracle.cbc-padding-oracle` and the "
"message from the code analysis is `Using CBC with PKCS5Padding is "
"susceptible to padding oracle attacks. A malicious actor could discern the "
"difference between plaintext with valid or invalid padding. Further, CBC "
"mode does not include any integrity checks. Use 'AES/GCM/NoPadding' "
"instead.`."
"```"
)


Expand Down
19 changes: 8 additions & 11 deletions tests/utils_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,16 @@ def testConstructTechnicalDetail_allDetailsProvided_returnsTechnicalDetail(

assert (
technical_detail
== "The file `tests/files/vulnerable.java` has a security issue at line `28`, column "
"`44`:\n"
== "Cbc Padding Oracle: Using CBC with PKCS5Padding is susceptible to padding "
"oracle attacks. A malicious actor could discern the difference between "
"plaintext with valid or invalid padding. Further, CBC mode does not include "
"any integrity checks. Use 'AES/GCM/NoPadding' instead.\n"
" \n"
"The issue was detected in `tests/files/vulnerable.java`, line `28`, column "
"`44`, below is a code snippet from the vulnerable code\n"
"```java\n"
"Cipher cipher = Cipher.getInstance('AES/CBC/PKCS5Padding');\n"
"```\n"
"\n"
"The issue was identified as "
"`java.lang.security.audit.cbc-padding-oracle.cbc-padding-oracle` and the "
"message from the code analysis is `Using CBC with PKCS5Padding is "
"susceptible to padding oracle attacks. A malicious actor could discern the "
"difference between plaintext with valid or invalid padding. Further, CBC "
"mode does not include any integrity checks. Use 'AES/GCM/NoPadding' "
"instead.`."
"```"
)


Expand Down