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

validator: company #623

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

validator: company #623

wants to merge 4 commits into from

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:

@bangarang bangarang changed the base branch from main to chore/addValidatorsFolder September 24, 2024 07:33
@flatfile-nullify
Copy link

flatfile-nullify bot commented Sep 24, 2024

Nullify Code Vulnerabilities

2 findings found in this pull request

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

You can find a list of all findings here

@bangarang bangarang changed the title feat/CompanyValidator validator: company Sep 24, 2024
Base automatically changed from chore/addValidatorsFolder to main September 25, 2024 18:56
@carlbrugger
Copy link
Contributor

Did not test address or EIN validation.

@bangarang
Copy link
Collaborator Author

Did not test address or EIN validation.

Wonder if we just test for the formatting of the EIN with something like this: https://www.npmjs.com/package/ein-validator
Feel like the idea of this one would be to ping an internal verification API for some kind of Company existing in their records vs if the EIN has been registered in the US 😂

carlbrugger
carlbrugger previously approved these changes Oct 4, 2024
Comment on lines +43 to +48
const response = await fetch(
`https://api.einverification.com/verify/${ein}`, //TODO: find a working API
{
headers: { Authorization: `Bearer ${apiKey}` },
}
)

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 ein input to ensure it only contains expected characters (digits in the case of an EIN). This is done using a regular expression. The apiKey is also securely managed and not exposed.

autoFixesExperimental

Validate the ein input and securely manage the apiKey

Suggested change
const response = await fetch(
`https://api.einverification.com/verify/${ein}`, //TODO: find a working API
{
headers: { Authorization: `Bearer ${apiKey}` },
}
)
if (!/^[0-9]+$/.test(ein)) {
throw new Error('Invalid EIN');
}
const secureApiKey = process.env.API_KEY;
const response = await fetch(
`https://api.einverification.com/verify/${ein}`, //TODO: find a working API
{
headers: { Authorization: `Bearer ${secureApiKey}` },
}
)

poweredByNullify

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

Comment on lines +62 to +67
try {
const secrets = await api.secrets.list({ spaceId, environmentId })
return secrets.data.find((secret) => secret.name === name)?.value
} catch (e) {
console.error(e, `Error fetching secret ${name}`)
}

Choose a reason for hiding this comment

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

Nullify Code Language: TypeScript 🔵 MEDIUM Severity CWE-209

Generic error disclosure

Error messages with stack traces may expose sensitive information about the application.

Here's how you might fix this potential vulnerability

In the initial code, the entire error object was being logged to the console, potentially exposing sensitive system information. The modified code mitigates this vulnerability by only logging the error message, which should provide enough information for debugging purposes without exposing sensitive details.

autoFixesExperimental

Replace the console.error statement to only log the error message instead of the entire error object.

Suggested change
try {
const secrets = await api.secrets.list({ spaceId, environmentId })
return secrets.data.find((secret) => secret.name === name)?.value
} catch (e) {
console.error(e, `Error fetching secret ${name}`)
}
console.error(`Error fetching secret ${name}. Error message: ${e.message}`)

poweredByNullify

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

@carlbrugger carlbrugger dismissed their stale review October 4, 2024 23:20

Missing working EIN API

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.

2 participants