Potential fix for code scanning alert no. 24: Server-side request forgery#29
Merged
Potential fix for code scanning alert no. 24: Server-side request forgery#29
Conversation
…gery Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: AU_gdev_19 <64915515+Dargon789@users.noreply.github.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements SSRF mitigation in $getAcceleratorAccelerationsStats by introducing an allow-list for service paths and validating user-supplied input before proxying requests. Sequence diagram for SSRF mitigation in $getAcceleratorAccelerationsStatssequenceDiagram
participant User as actor User
participant API as AccelerationRoutes
participant Logger
participant Service as MEMPOOL_SERVICES.API
User->>API: Request to /api/v1/services/{userPath}
API->>API: Strip '/api/v1/services/' prefix
API->>API: Check if userPath is in allowedPaths
alt Path allowed
API->>Service: GET /{safePath}
Service-->>API: Response
API-->>User: Return service response
else Path not allowed
API->>Logger: Log invalid path
API-->>User: 400 error (Invalid path)
end
Class diagram for updated AccelerationRoutes SSRF mitigationclassDiagram
class AccelerationRoutes {
+$getAcceleratorAccelerationsStats(req: Request, res: Response): Promise<void>
-allowedPaths: {accelerations, accelerations/history, accelerations/stats, estimate}
}
AccelerationRoutes --> "uses" logger: Logger
AccelerationRoutes --> "uses" config: Config
AccelerationRoutes --> "uses" axios: Axios
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Potential fix for https://github.com/Dargon789/mempool/security/code-scanning/24
To fix this SSRF vulnerability, we must ensure that user-supplied data cannot arbitrarily control the target of the server-side HTTP request. As seen in other functions like
$getAcceleratorAccelerationsHistoryAggregatedand$getAcceleratorEstimate, the code uses anallowedPathsallow-list to restrict which paths may be proxied/fetched based on user input, mapping user-supplied input to a controlled, known set of API endpoints. We should replicate this pattern in$getAcceleratorAccelerationsStats.Specifically:
allowedPathsmapping specifying valid relative API endpoints./api/v1/services/prefix, and check if it exists in the allow-list.$getAcceleratorAccelerationsStatsmethod require editing.Suggested fixes powered by Copilot Autofix. Review carefully before merging.
Summary by Sourcery
Bug Fixes: