-
Notifications
You must be signed in to change notification settings - Fork 11
Feat/strict typing #683
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
base: main
Are you sure you want to change the base?
Feat/strict typing #683
Conversation
contraints tests broken, skipping them
| ): Promise<SetupFactory> { | ||
| const schemas = await getSchemas(models) | ||
| const asdf = schemas.map((schema) => jsYaml.load(schema)) | ||
| const jsonSchemas = schemas.map((schema) => jsYaml.load(schema)) |
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.
Nullify Code Language: TypeScript 🟡 HIGH Severity CWE-502
Rules lgpl javascript eval rule yaml deserialize
The vulnerability exists in the generateSetup function where jsYaml.load() is used to parse YAML data fetched from external URLs without proper sanitization or validation. This can lead to Remote Code Injection if an attacker can control the YAML content. The parsed data is used to generate application configuration, which means an attacker could potentially manipulate the entire application's behavior.
⚡ Here's how you might fix this potential vulnerability
The initial code was using 'jsYaml.load' to parse YAML data, which can execute arbitrary JavaScript code if the YAML data contains malicious content. This can lead to Remote Code Execution (RCE) if an attacker can control the input YAML data. The modified code mitigates this vulnerability by using 'jsYaml.safeLoad' instead of 'jsYaml.load'. The 'safeLoad' function is a safe alternative that does not allow executing JavaScript code.
autoFixesExperimental
Replace 'jsYaml.load' with 'jsYaml.safeLoad' to prevent execution of arbitrary JavaScript code.
| const jsonSchemas = schemas.map((schema) => jsYaml.load(schema)) | |
| const jsonSchemas = schemas.map((schema) => jsYaml.safeLoad(schema)) |
poweredByNullify
Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)
| const response = await fetch(webhookUrl, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| }), | ||
| }) | ||
| body: JSON.stringify({ | ||
| workbook: { | ||
| ...workbook, | ||
| sheets, | ||
| }, | ||
| }), | ||
| }) |
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.
Nullify Code Language: TypeScript 🟡 HIGH Severity CWE-918
Rules lgpl javascript ssrf rule node ssrf
The webhookEgress function in plugins/webhook-egress/src/webhook.egress.ts accepts a webhookUrl parameter without any validation or sanitization. This parameter is then directly used in a fetch function call, which can lead to Server-Side Request Forgery (SSRF). An attacker could manipulate the webhookUrl to make requests to internal systems or services that should not be accessible externally, potentially exposing sensitive information or enabling further attacks.
⚡ Here's how you might fix this potential vulnerability
The modified code adds a validation step before making the request. It uses the URL class in Node.js to parse the URL and check its protocol, host, and port. Only http and https protocols are allowed, and URLs with localhost, 127.0.0.1, or any other internal network IP as the host are blocked. Also, any non-standard ports are blocked. This ensures that the URL is valid and safe before making the request, effectively mitigating the SSRF vulnerability.
autoFixesExperimental
Add URL validation before making the request
| const response = await fetch(webhookUrl, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| }), | |
| }) | |
| body: JSON.stringify({ | |
| workbook: { | |
| ...workbook, | |
| sheets, | |
| }, | |
| }), | |
| }) | |
| const url = new URL(webhookUrl); | |
| if (url.protocol !== 'http:' && url.protocol !== 'https:') { | |
| throw new Error('Invalid protocol'); | |
| } | |
| if (url.hostname === 'localhost' || url.hostname === '127.0.0.1') { | |
| throw new Error('Invalid host'); | |
| } | |
| if (url.port && !['80', '443'].includes(url.port)) { | |
| throw new Error('Invalid port'); | |
| } | |
| const response = await fetch(webhookUrl, { |
Import the URL class from Node.js
Line 8
import { responseRejectionHandler } from '@flatfile/util-response-rejection'
import { URL } from 'url'poweredByNullify
Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)
Nullify Code Vulnerabilities3 findings found in this pull request
You can find a list of all findings here |
d27f1c0 to
449301d
Compare
Please explain how to summarize this PR for the Changelog:
Tell code reviewer how and what to test: