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.

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

mapping-utils: Strava GPX Fetcher #632

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

Conversation

bangarang
Copy link
Collaborator

  • chore: add /validators folder
  • koala: initial commit

Please explain how to summarize this PR for the Changelog:

Tell code reviewer how and what to test:

Comment on lines +57 to +62
const response = await axios.get(
`https://www.strava.com/api/v3/activities/${activityId}/streams?keys=latlng,altitude,time&key_by_type=true`,
{
headers: { Authorization: `Bearer ${accessToken}` },
}
)

Choose a reason for hiding this comment

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

Nullify Code Language: TypeScript 🟡 HIGH Severity CWE-918

Rules lgpl javascript ssrf rule node ssrf

This application allows user-controlled URLs to be passed directly to HTTP client libraries. This can result in Server-Side Request Forgery (SSRF). SSRF refers to an attack where the attacker can abuse functionality on the server to force it to make requests to other internal systems within your infrastructure that are not directly exposed to the internet. This allows the attacker to access internal resources they do not have direct access to.
Some risks of SSRF are:

  • Access and manipulation of internal databases, APIs, or administrative panels - Ability to scan internal network architecture and services - Can be used to pivot attacks into the internal network - Circumvent network segregation and firewall rules
    To avoid this, try using hardcoded HTTP request calls or a whitelisting object to check whether the user input is trying to access allowed resources or not.
    Here is an example: var whitelist = [ "https://example.com", "https://example.com/sample" ] app.get('/ssrf/node-ssrf/axios/safe/3', function (req, res) { if(whitelist.includes(req.query.url)){ axios.get(url, {}) .then(function (response) { console.log(response); }) .catch(function (response) { console.log(response); }) } }); For more information on SSRF see OWASP: https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html

Here's how you might fix this potential vulnerability

The modified code mitigates this vulnerability by validating the activityId before it is used in the axios.get request. It checks if the activityId is a positive integer, which is the expected format for a Strava activity ID. If the activityId is not a positive integer, the function throws an error. This ensures that only valid activity IDs can be used in the request, preventing SSRF attacks.

autoFixesExperimental

Add validation for activityId to prevent SSRF attacks

Suggested change
const response = await axios.get(
`https://www.strava.com/api/v3/activities/${activityId}/streams?keys=latlng,altitude,time&key_by_type=true`,
{
headers: { Authorization: `Bearer ${accessToken}` },
}
)
if (!Number.isInteger(activityId) || activityId <= 0) {
throw new Error('Invalid activity ID');
}
const response = await axios.get(
`https://www.strava.com/api/v3/activities/${activityId}/streams?keys=latlng,altitude,time&key_by_type=true`,
{
headers: { Authorization: `Bearer ${accessToken}` },
}
)

poweredByNullify

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)

@flatfile-nullify
Copy link

Nullify Code Vulnerabilities

1 findings found in this pull request

🔴 CRITICAL 🟡 HIGH 🔵 MEDIUM ⚪ LOW
0 1 0 0

You can find a list of all findings here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant