Skip to content

test semgrep for directly-returned-format-string and nan-injection #1

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

test semgrep for directly-returned-format-string and nan-injection #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
639eafb
test semgrep for directly-returned-format-string and nan-injection
alexcoderabbitai Oct 11, 2024
791cad9
trigger with linter
alexcoderabbitai Oct 11, 2024
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
4 changes: 0 additions & 4 deletions .github/workflows/linters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ name: Linters
on:
workflow_dispatch:
pull_request:
branches:
- main
push:
branches:
- main

jobs:
# TODO: Fix Bandit Vulns
Expand Down
50 changes: 1 addition & 49 deletions app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,55 +446,7 @@ def megaitemnames():
)
)


@app.route("/petshoppinglist", methods=["GET", "POST"])
def petshoppinglist():
return redirect("https://saddlebagexchange.com/wow/shopping-list")

# DEPRECIATED
if request.method == "GET":
return return_safe_html(render_template("petshoppinglist.html"))
elif request.method == "POST":
json_data = {
"region": request.form.get("region"),
"itemID": int(request.form.get("petID")),
"maxPurchasePrice": int(request.form.get("maxPurchasePrice")),
"connectedRealmIDs": {},
}

response = requests.post(
f"{api_url}/wow/shoppinglistx",
headers={"Accept": "application/json"},
json=json_data,
).json()

if "data" not in response:
logger.error(
f"Error no matching data with given inputs {json_data} response {response}"
)
if NO_RATE_LIMIT:
return f"Error no matching data with given inputs {json_data} response {response}"
# send generic error message to remove XSS potential
return f"error no matching results found matching search inputs"

response = response["data"]

column_order = [
"realmID",
"price",
"quantity",
"realmName",
"realmNames",
"link",
]
response = [{key: item.get(key) for key in column_order} for item in response]
fieldnames = list(response[0].keys())

return return_safe_html(
render_template(
"petshoppinglist.html", results=response, fieldnames=fieldnames, len=len
)
)



@app.route("/petmarketshare", methods=["GET", "POST"])
Expand Down
53 changes: 53 additions & 0 deletions routes/wow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,57 @@ def wow_outofstock_api():
fieldnames=fieldnames,
len=len,
)
)

@wow_bp.route("/petshoppinglist", methods=["GET", "POST"])
def petshoppinglist():
# return redirect("https://saddlebagexchange.com/wow/shopping-list")

# DEPRECIATED
if request.method == "GET":
return return_safe_html(render_template("petshoppinglist.html"))
elif request.method == "POST":
json_data = {
"region": request.form.get("region"),
"itemID": int(request.form.get("petID")),
"maxPurchasePrice": int(request.form.get("maxPurchasePrice")),
Comment on lines +105 to +106
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle potential exceptions when converting form data to integers

Using int() directly on form data may raise a ValueError if the input is not a valid integer. It's recommended to validate the input and handle possible exceptions to avoid server errors.

Consider adding try-except blocks or input validation. For example:

try:
    item_id = int(request.form.get("petID"))
except (ValueError, TypeError):
    return return_safe_html("Invalid petID provided.")

try:
    max_purchase_price = int(request.form.get("maxPurchasePrice"))
except (ValueError, TypeError):
    return return_safe_html("Invalid maxPurchasePrice provided.")

json_data = {
    "region": request.form.get("region"),
    "itemID": item_id,
    "maxPurchasePrice": max_purchase_price,
    "connectedRealmIDs": {},
}

"connectedRealmIDs": {},
}

print(json_data)

response = requests.post(
f"{api_url}/wow/shoppinglistx",
headers={"Accept": "application/json"},
json=json_data,
).json()

print(response)

if "data" not in response:
print(
f"Error no matching data with given inputs {json_data} response {response}"
)
if NO_RATE_LIMIT:
return f"Error no matching data with given inputs {json_data} response {response}"
# send generic error message to remove XSS potential
return f"error no matching results found matching search inputs"

Comment on lines +120 to +128
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve error handling to prevent XSS vulnerabilities and ensure consistent error responses

Returning formatted strings that include user input directly can lead to cross-site scripting (XSS) vulnerabilities if the input is not properly sanitized. In this block, error messages include json_data and response, which may contain user input.

Consider standardizing the error responses using safe rendering methods and avoiding inclusion of user-provided data. Apply this diff to fix the issue:

             if NO_RATE_LIMIT:
-                return f"Error no matching data with given inputs {json_data} response {response}"
+                return return_safe_html("Error: no matching data found for the provided inputs.")

             # send generic error message to remove XSS potential
-            return f"error no matching results found matching search inputs"
+            return return_safe_html("Error: no matching results found matching search inputs.")

Also, remove the unnecessary f prefix in line 127 as there are no placeholders in the string.

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 Ruff

127-127: f-string without any placeholders

Remove extraneous f prefix

(F541)

🪛 semgrep

[WARNING] 125-125: [WARNING] 125-125: Detected Flask route directly returning a formatted string. This is subject to cross-site scripting if user input can reach the string. Consider using the template engine instead and rendering pages with 'render_template()'.

response = response["data"]

column_order = [
"realmID",
"price",
"quantity",
"realmName",
"realmNames",
"link",
]
response = [{key: item.get(key) for key in column_order} for item in response]
fieldnames = list(response[0].keys())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle potential empty response when accessing response[0]

At line 140, fieldnames = list(response[0].keys()) assumes that response is a non-empty list. If response is empty, this will raise an IndexError.

Consider checking if response is not empty before accessing response[0]. For example:

if response:
    fieldnames = list(response[0].keys())
else:
    return return_safe_html("No data available for the given inputs.")


return return_safe_html(
render_template(
"petshoppinglist.html", results=response, fieldnames=fieldnames, len=len
)
)
Loading
Loading