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: spell checker #625

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

validator: spell checker #625

wants to merge 2 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:

const affPath =
affixPath || path.join(__dirname, `dictionaries/${language}.aff`)

const dictionary = fs.readFileSync(dicPath, 'utf-8')

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-22

Javascript pathtraversal rule non literal fs filename

The application dynamically constructs file or path information. If the path
information comes from user-supplied input, it could be abused to read sensitive files,
access other users' data, or aid in exploitation to gain further system access.

User input should never be used in constructing paths or files for interacting
with the filesystem. This includes filenames supplied by user uploads or downloads.
If possible, consider hashing user input or using unique values and
use path.normalize to resolve and validate the path information
prior to processing any file functionality.

Example using path.normalize and not allowing direct user input:

// User input, saved only as a reference
// id is a randomly generated UUID to be used as the filename
const userData = {userFilename: userSuppliedFilename, id: crypto.randomUUID()};
// Restrict all file processing to this directory only
const basePath = '/app/restricted/';

// Create the full path, but only use our random generated id as the filename
const joinedPath = path.join(basePath, userData.id);
// Normalize path, removing any '..'
const fullPath = path.normalize(joinedPath);
// Verify the fullPath is contained within our basePath
if (!fullPath.startsWith(basePath)) {
    console.log("Invalid path specified!");
}
// Process / work with file
// ...

For more information on path traversal issues see OWASP:
https://owasp.org/www-community/attacks/Path_Traversal

Here's how you might fix this potential vulnerability

The modified code mitigates this vulnerability by sanitizing the language variable before using it to construct the file path. It does this by using the path.basename function, which removes any directory paths from the input, ensuring that the resulting path is always within the intended directory. This prevents an attacker from being able to traverse outside of the intended directory.

autoFixesExperimental

Sanitize the language variable before using it to construct the file path
Lines 41-44

  const sanitizedLanguage = path.basename(language)
  const dicPath =
    dictionaryPath || path.join(__dirname, `dictionaries/${sanitizedLanguage}.dic`)
  const affPath =
    affixPath || path.join(__dirname, `dictionaries/${sanitizedLanguage}.aff`)

poweredByNullify

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

affixPath || path.join(__dirname, `dictionaries/${language}.aff`)

const dictionary = fs.readFileSync(dicPath, 'utf-8')
const affixFile = fs.readFileSync(affPath, 'utf-8')

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-22

Javascript pathtraversal rule non literal fs filename

The application dynamically constructs file or path information. If the path
information comes from user-supplied input, it could be abused to read sensitive files,
access other users' data, or aid in exploitation to gain further system access.

User input should never be used in constructing paths or files for interacting
with the filesystem. This includes filenames supplied by user uploads or downloads.
If possible, consider hashing user input or using unique values and
use path.normalize to resolve and validate the path information
prior to processing any file functionality.

Example using path.normalize and not allowing direct user input:

// User input, saved only as a reference
// id is a randomly generated UUID to be used as the filename
const userData = {userFilename: userSuppliedFilename, id: crypto.randomUUID()};
// Restrict all file processing to this directory only
const basePath = '/app/restricted/';

// Create the full path, but only use our random generated id as the filename
const joinedPath = path.join(basePath, userData.id);
// Normalize path, removing any '..'
const fullPath = path.normalize(joinedPath);
// Verify the fullPath is contained within our basePath
if (!fullPath.startsWith(basePath)) {
    console.log("Invalid path specified!");
}
// Process / work with file
// ...

For more information on path traversal issues see OWASP:
https://owasp.org/www-community/attacks/Path_Traversal

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

let correctedText = text
suggestions.forEach(({ word, suggestions }) => {
if (suggestions.length > 0) {
const regex = new RegExp(`\\b${word}\\b`, 'g')

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-185

Javascript dos rule non literal regexp

The RegExp constructor was called with a non-literal value. If an adversary were able to
supply a malicious regex, they could cause a Regular Expression Denial of Service (ReDoS)
against the application. In Node applications, this could cause the entire application to no
longer be responsive to other users' requests.

To remediate this issue, never allow user-supplied regular expressions. Instead, the regular
expression should be hardcoded. If this is not possible, consider using an alternative regular
expression engine such as node-re2. RE2 is a safe alternative
that does not support backtracking, which is what leads to ReDoS.

Example using re2 which does not support backtracking (Note: it is still recommended to
never use user-supplied input):

// Import the re2 module
const RE2 = require('re2');

function match(userSuppliedRegex, userInput) {
    // Create a RE2 object with the user supplied regex, this is relatively safe
    // due to RE2 not supporting backtracking which can be abused to cause long running
    // queries
    var re = new RE2(userSuppliedRegex);
    // Execute the regular expression against some userInput
    var result = re.exec(userInput);
    // Work with the result
}

For more information on Regular Expression DoS see:

Here's how you might fix this potential vulnerability

In the initial code, the 'word' variable is directly used in the RegExp constructor, without any sanitization. This can lead to Regular Expression Denial of Service (ReDoS) if the 'word' variable contains special characters that have special meaning in a regular expression. The modified code mitigates this vulnerability by escaping any special characters in 'word' before using it in the RegExp constructor. This ensures that 'word' is treated as a literal string within the regular expression, and not as part of the regular expression itself.

autoFixesExperimental

Escape special characters in 'word' before using it in the RegExp constructor to prevent ReDoS.

Suggested change
const regex = new RegExp(`\\b${word}\\b`, 'g')
const escapedWord = word.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const regex = new RegExp(`\\b${escapedWord}\\b`, 'g')

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

3 findings found in this pull request

🔴 CRITICAL 🟡 HIGH 🔵 MEDIUM ⚪ LOW
0 0 3 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