Major updates, rework#17
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| const mimetype = file.mimetype; | ||
| const buffer = await file.toBuffer(); | ||
| console.log(ingredientsArray); |
Check warning
Code scanning / CodeQL
Log injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 months ago
The best way to fix the problem is to sanitize any user-provided input before logging it. Since ingredientsArray is derived from user input, and could contain strings with newlines or control characters, all elements of ingredientsArray should be sanitized prior to logging. The solution is to map each element of the array (assuming they are strings) and replace any \n or \r characters with an empty string (if not strings, optionally coerce them using String(value)). Then, log the sanitized array instead.
Specifically:
- Replace the current
console.log(ingredientsArray);on line 74 with sanitized logging. - Optionally add a comment explaining the sanitization for future maintainers.
- No need for extra imports; use built-in string methods.
| @@ -71,7 +71,15 @@ | ||
|
|
||
| const mimetype = file.mimetype; | ||
| const buffer = await file.toBuffer(); | ||
| console.log(ingredientsArray); | ||
| // Sanitize each ingredient to prevent log injection | ||
| const sanitizedIngredientsArray = Array.isArray(ingredientsArray) | ||
| ? ingredientsArray.map(val => | ||
| typeof val === 'string' | ||
| ? val.replace(/[\n\r]/g, "") | ||
| : val | ||
| ) | ||
| : []; | ||
| console.log(sanitizedIngredientsArray); | ||
|
|
||
| await createToothpaste({ | ||
| name: name, |
| }; | ||
| } else { | ||
| console.log( | ||
| ` + Adding new ingredient: ${ingredientName}, existing: ${existingIngredient}`, |
Check warning
Code scanning / CodeQL
Log injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 months ago
To fix the issue, we need to ensure that any user-controlled input included in log messages cannot inject new lines or special log-breaking characters. The recommended mitigation is to sanitize all such user inputs before logging, most simply by stripping newlines and carriage returns from those values using String.prototype.replace. We should update every log statement that outputs potentially user-sourced data -- especially ingredientName -- to log a sanitized version. This edit will only affect the logging, not the program's logic or persisted data.
Specifically, in src/services/ingredient.extractor.service.js:
- Strip all
\rand\ncharacters fromingredientNamewhen used directly in log output on lines 17 and 39. - Optionally, clearly delimit or bracket the sanitized user input as recommended for clarity.
No new packages or external dependencies are required for this standard sanitization.
| @@ -13,8 +13,9 @@ | ||
|
|
||
| for (let i = 0; i < ingredientsText.length; i++) { | ||
| const ingredientName = ingredientsText[i]; | ||
| const safeIngredientName = typeof ingredientName === "string" ? ingredientName.replace(/[\r\n]/g, "") : String(ingredientName); | ||
|
|
||
| console.log(`Processing ingredient: ${ingredientName}`); | ||
| console.log(`Processing ingredient: [${safeIngredientName}]`); | ||
|
|
||
| const existingIngredient = await fuzzyFindSimilarIngredient(ingredientName); | ||
|
|
||
| @@ -36,7 +36,7 @@ | ||
| }; | ||
| } else { | ||
| console.log( | ||
| ` + Adding new ingredient: ${ingredientName}, existing: ${existingIngredient}`, | ||
| ` + Adding new ingredient: [${safeIngredientName}], existing: ${existingIngredient}`, | ||
| ); | ||
| const newIngredient = await insertIngredient(ingredientName); | ||
| ingredientData = { |
2 participants
Completed the implementation of a functional version of the application.
This version is integrated with PastePick-py-server and PastePick-frontend.
Key features
Closes #7
Closes #8
Closes #9
Closes #10
Closes #11
Closes #12
Closes #13
Closes #14
Closes #15
Closes #16