-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Proxy #1
Proxy #1
Conversation
except Exception as e: | ||
msg = f"Failed to decompress data: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg.encode(), status=400) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 2 months ago
To fix the problem, we need to ensure that detailed exception messages are not exposed to the end user. Instead, we should log the detailed error message on the server and return a generic error message to the user. This can be achieved by modifying the exception handling blocks to log the detailed error and return a generic message.
Steps to fix:
- Modify the exception handling block on lines 103-106 to log the detailed error message and return a generic error message.
- Similarly, modify the exception handling block on lines 111-114 to log the detailed error message and return a generic error message.
-
Copy modified lines R104-R105 -
Copy modified lines R111-R112
@@ -103,5 +103,4 @@ | ||
except Exception as e: | ||
msg = f"Failed to decompress data: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg.encode(), status=400) | ||
logger.debug(f"Failed to decompress data: {str(e)}") | ||
return HttpResponse("Failed to decompress data.", status=400) | ||
|
||
@@ -111,5 +110,4 @@ | ||
except Exception as e: | ||
msg = f"Failed to decode metrics: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg, status=400) | ||
logger.debug(f"Failed to decode metrics: {str(e)}") | ||
return HttpResponse("Failed to decode metrics.", status=400) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👆
except Exception as e: | ||
msg = f"Failed to decode metrics: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg, status=400) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 2 months ago
To fix the problem, we need to ensure that the error messages returned to the user do not contain any sensitive information. Instead of including the exception message (str(e)
) in the response, we should return a generic error message. The detailed error information should be logged on the server for debugging purposes.
Specifically, we will:
- Replace the detailed error messages in the
HttpResponse
with a generic message. - Ensure that the detailed error information is logged using
logger.debug
.
-
Copy modified lines R104-R105 -
Copy modified lines R111-R112
@@ -103,5 +103,4 @@ | ||
except Exception as e: | ||
msg = f"Failed to decompress data: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg.encode(), status=400) | ||
logger.debug(f"Failed to decompress data: {str(e)}") | ||
return HttpResponse("Failed to decompress data.", status=400) | ||
|
||
@@ -111,5 +110,4 @@ | ||
except Exception as e: | ||
msg = f"Failed to decode metrics: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg, status=400) | ||
logger.debug(f"Failed to decode metrics: {str(e)}") | ||
return HttpResponse("Failed to decode metrics.", status=400) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes I agree with this, but the code blocks under "try" Are terribly short in these cases
hotkey = label.value | ||
if label.value != ss58_address: | ||
msg = f"Received invalid hotkey. Expected {ss58_address} got {label.value}" | ||
error = HttpResponse(status=403, content=msg.encode()) |
Check warning
Code scanning / CodeQL
Reflected server-side cross-site scripting Medium
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 2 months ago
To fix the problem, we need to ensure that any user-provided input included in the HTTP response is properly sanitized or escaped to prevent XSS attacks. In this case, we can use the html.escape()
function from the html
module to escape the user input before including it in the response. This will convert special characters to their corresponding HTML entities, preventing the browser from interpreting them as executable code.
We will apply this fix to the relevant lines in the prometheus_inbound_proxy
function where user input is included in the msg
variable.
-
Copy modified line R2 -
Copy modified line R105 -
Copy modified line R113 -
Copy modified line R130 -
Copy modified line R135 -
Copy modified line R139
@@ -1,2 +1,3 @@ | ||
from urllib.parse import urljoin | ||
import html | ||
|
||
@@ -103,3 +104,3 @@ | ||
except Exception as e: | ||
msg = f"Failed to decompress data: {str(e)}" | ||
msg = f"Failed to decompress data: {html.escape(str(e))}" | ||
logger.debug(msg) | ||
@@ -111,3 +112,3 @@ | ||
except Exception as e: | ||
msg = f"Failed to decode metrics: {str(e)}" | ||
msg = f"Failed to decode metrics: {html.escape(str(e))}" | ||
logger.debug(msg) | ||
@@ -128,3 +129,3 @@ | ||
if label.value != ss58_address: | ||
msg = f"Received invalid hotkey. Expected {ss58_address} got {label.value}" | ||
msg = f"Received invalid hotkey. Expected {html.escape(ss58_address)} got {html.escape(label.value)}" | ||
error = HttpResponse(status=403, content=msg.encode()) | ||
@@ -133,3 +134,3 @@ | ||
if not hotkey: | ||
msg = f"Received no hotkey" | ||
msg = "Received no hotkey" | ||
error = HttpResponse(status=403, content=msg.encode()) | ||
@@ -137,3 +138,3 @@ | ||
if error is not None: | ||
error.content = f"Metric: {name}. ".encode() + error.content | ||
error.content = f"Metric: {html.escape(name)}. ".encode() + error.content | ||
logger.info(error.content.decode()) |
logger.debug(msg) | ||
return HttpResponse(status=400, content=msg) | ||
|
||
if ss58_address not in Validator.objects.filter(active=True).values_list('public_key', flat=True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ss58_address not in Validator.objects.filter(active=True).values_list('public_key', flat=True): | |
if not Validator.objects.filter(active=True, public_key=ss58_address).exists(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's gonna nuke the caches cache with each new request, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I missed the use of cacheops.
except Exception as e: | ||
msg = f"Failed to decompress data: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg.encode(), status=400) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👆
except Exception as e: | ||
msg = f"Failed to decode metrics: {str(e)}" | ||
logger.debug(msg) | ||
return HttpResponse(msg, status=400) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👆
Co-authored-by: emnoor-reef <137923473+emnoor-reef@users.noreply.github.com>
Co-authored-by: emnoor-reef <137923473+emnoor-reef@users.noreply.github.com>
No description provided.