Add software list export#155
Conversation
Signed-off-by: Mark Rivera <mark.c.rivera@lmco.com>
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for (const myVal of splitted) { | ||
| // allObjects += myVal.substring(0, myVal.indexOf("\n")) + "\n"; | ||
| var softwareDetails = myVal.split("\n"); | ||
| var mySoftware = softwareDetails[0].replace('"', '"'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
Copilot Autofix
AI 5 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| var myVersion = myVal | ||
| .split("All Possible Versions")[1] | ||
| .split("\n")[0] | ||
| .split(":")[1] | ||
| .trim() | ||
| .replace('"', '"'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
General approach: when escaping characters in strings, either use a well-tested library or ensure that replacements cover all occurrences, typically using a regular expression with the g flag. Also, escape backslashes correctly where relevant. In this snippet, the two .replace('"', '"') calls are both incomplete and misleading; if they were changed later to something like .replace('"', '""'), they would only escape the first quote. The best fix is to either (1) remove these no-op replacements entirely, or (2) convert them to a correct global replacement that will work if the code is later updated to do real escaping.
Best minimal fix without changing functionality: since the current replacements do nothing (" -> "), removing them would not change runtime behavior at all. However, the intention appears to be to escape internal quotes before wrapping the values in double quotes for printName and printVersion. To both preserve current behavior and make the code robust for its likely intended purpose, I will replace the current replace calls with global-regex-based replacements that still map " to " (thus still a no-op) but follow the safe pattern. If the project later changes the replacement string to something else (e.g. \" or ""), all occurrences will then be handled correctly.
Concretely:
- In
server/utils/excelExport/nessusExport.ts, inside the branchif (pluginId == 178102):- Change
var mySoftware = softwareDetails[0].replace('"', '"');to use a global regex:replace(/"/g, '"'). - Change the final
.replace('"', '"')in themyVersionchain to.replace(/"/g, '"').
- Change
No new methods or imports are needed; this relies only on built-in String.prototype.replace with a regex.
| @@ -216,14 +216,14 @@ | ||
| for (const myVal of splitted) { | ||
| // allObjects += myVal.substring(0, myVal.indexOf("\n")) + "\n"; | ||
| var softwareDetails = myVal.split("\n"); | ||
| var mySoftware = softwareDetails[0].replace('"', '"'); | ||
| var mySoftware = softwareDetails[0].replace(/"/g, '"'); | ||
| if (mySoftware != "") { | ||
| var myVersion = myVal | ||
| .split("All Possible Versions")[1] | ||
| .split("\n")[0] | ||
| .split(":")[1] | ||
| .trim() | ||
| .replace('"', '"'); | ||
| .replace(/"/g, '"'); | ||
| var printName = '"' + mySoftware + '"'; | ||
| var printVersion = '"' + myVersion + '"'; | ||
| var theseSystems = addSoftwareSystem( |
| for (const myVal of splitted) { | ||
| // allObjects += myVal.substring(0, myVal.indexOf("\n")) + "\n"; | ||
| var softwareDetails = myVal.split("\n"); | ||
| var mySoftware = softwareDetails[0].replace('"', '"'); |
Check warning
Code scanning / CodeQL
Replacement of a substring with itself Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
In general, to fix “replacement of a substring with itself” you either (1) correct the replacement string to perform the intended transformation (e.g., escape or strip characters), or (2) remove the call entirely if no transformation is needed. Here, we do not have enough evidence of a specific intended transformation (escaping vs. stripping), but we know for certain the current replace('"', '"') calls are redundant. Removing them does not change runtime behavior (since they are already no-ops) but eliminates the defect and clarifies the code.
Concretely, in server/utils/excelExport/nessusExport.ts, within the if (pluginId == 178102) branch:
- On line 219, replace
var mySoftware = softwareDetails[0].replace('"', '"');withvar mySoftware = softwareDetails[0];. - On line 226, replace the chained
.replace('"', '"')with nothing, i.e., letmyVersionbe the trimmed substring result directly.
No new imports, methods, or definitions are required; we are just simplifying existing expressions.
| @@ -216,14 +216,13 @@ | ||
| for (const myVal of splitted) { | ||
| // allObjects += myVal.substring(0, myVal.indexOf("\n")) + "\n"; | ||
| var softwareDetails = myVal.split("\n"); | ||
| var mySoftware = softwareDetails[0].replace('"', '"'); | ||
| var mySoftware = softwareDetails[0]; | ||
| if (mySoftware != "") { | ||
| var myVersion = myVal | ||
| .split("All Possible Versions")[1] | ||
| .split("\n")[0] | ||
| .split(":")[1] | ||
| .trim() | ||
| .replace('"', '"'); | ||
| .trim(); | ||
| var printName = '"' + mySoftware + '"'; | ||
| var printVersion = '"' + myVersion + '"'; | ||
| var theseSystems = addSoftwareSystem( |
| .split("\n")[0] | ||
| .split(":")[1] | ||
| .trim() | ||
| .replace('"', '"'); |
Check warning
Code scanning / CodeQL
Replacement of a substring with itself Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
In general, to fix this kind of problem you must ensure the replacement string is actually different from the search string and matches the intended transformation (e.g., escaping, normalization, or removal). Here the apparent goal is to safely embed software names and versions inside double quotes for output, which requires escaping any existing double quotes inside those strings.
The best targeted fix without changing existing behavior (except correcting this bug) is:
- Change the
.replace('"', '"')calls on bothmySoftwareandmyVersionto escape embedded quotes by replacing each"with"", which is the standard way to escape quotes inside double-quoted fields in CSV/Excel contexts. - Use a global replacement so all occurrences of
"are handled, not just the first one.
Concretely in server/utils/excelExport/nessusExport.ts:
- At line 219, change
var mySoftware = softwareDetails[0].replace('"', '"');tovar mySoftware = softwareDetails[0].replace(/"/g, '""');. - At line 226, change
.replace('"', '"');to.replace(/"/g, '""');.
No new imports or helper methods are needed; this uses built-in JavaScript String.prototype.replace with a regex literal.
| @@ -216,14 +216,14 @@ | ||
| for (const myVal of splitted) { | ||
| // allObjects += myVal.substring(0, myVal.indexOf("\n")) + "\n"; | ||
| var softwareDetails = myVal.split("\n"); | ||
| var mySoftware = softwareDetails[0].replace('"', '"'); | ||
| var mySoftware = softwareDetails[0].replace(/"/g, '""'); | ||
| if (mySoftware != "") { | ||
| var myVersion = myVal | ||
| .split("All Possible Versions")[1] | ||
| .split("\n")[0] | ||
| .split(":")[1] | ||
| .trim() | ||
| .replace('"', '"'); | ||
| .replace(/"/g, '""'); | ||
| var printName = '"' + mySoftware + '"'; | ||
| var printVersion = '"' + myVersion + '"'; | ||
| var theseSystems = addSoftwareSystem( |
3 participants
No description provided.