Skip to content

Commit

Permalink
Merge pull request #12 from Ostorlab/improve_technical_details
Browse files Browse the repository at this point in the history 
Improve technical details
  • Loading branch information
@BlueSquare1
BlueSquare1 authored Aug 2, 2023
2 parents 971d0da + a68be80 commit 71834ed
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 49 deletions.
14 changes: 8 additions & 6 deletions agent/utils.py
View file
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
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
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
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

0 comments on commit 71834ed

Please sign in to comment.