Skip to content

Fix multiple injection vulnerabilities in database queries, code execution, and file paths - #36

Open
sonarqube-agent[bot] wants to merge 2 commits into
masterfrom
remediate-master-20260615-010129-12cdefa3
Open

sonarqube-agent[bot] wants to merge 2 commits into
masterfrom
remediate-master-20260615-010129-12cdefa3

Conversation

@sonarqube-agent

Copy link
Copy Markdown

This PR was automatically created by the Remediation Agent's Scheduled backlog remediation feature.

Why these issues? Selected for their BLOCKER severity and well-defined injection-prevention patterns across diverse security domains (NoSQL, SQL, code execution, and path traversal). Each fix employs standard defensive techniques (parameterization, safe operators, path canonicalization) with clear mechanical implementations, maximizing automation accuracy and reviewability across security-critical code paths.

This PR fixes 5 critical injection vulnerabilities including NoSQL injection in order placement, MongoDB code injection in product reviews, SQL injection in login, and path traversal in file operations. All fixes prevent attackers from manipulating database queries, executing arbitrary code, or accessing unauthorized files through user-controlled input.

View Project in SonarCloud


Fixed Issues

tssecurity:S5147 - Change this code to not construct database queries directly from user-controlled data. • BLOCKERView issue

Location: routes/order.ts:154

Why is this an issue?

NoSQL injections occur when an application retrieves untrusted data and inserts it into a database query without sanitizing it first.

What changed

This hunk fixes a NoSQL injection vulnerability in the order placement route. The original code directly inserted user-controlled values (req.body.orderDetails.paymentId and req.body.orderDetails.addressId) into a MongoDB insert operation without sanitization. An attacker could craft these fields as objects (e.g., {$ne: ""}) instead of plain strings, potentially manipulating the database query. By wrapping both values with String(), the fix ensures that regardless of what type the user submits (object, array, etc.), the value inserted into the database will always be a plain string, preventing NoSQL injection attacks.

--- a/routes/order.ts
+++ b/routes/order.ts
@@ -156,2 +156,2 @@ module.exports = function placeOrder () {
-            paymentId: req.body.orderDetails ? req.body.orderDetails.paymentId : null,
-            addressId: req.body.orderDetails ? req.body.orderDetails.addressId : null,
+            paymentId: req.body.orderDetails ? String(req.body.orderDetails.paymentId) : null,
+            addressId: req.body.orderDetails ? String(req.body.orderDetails.addressId) : null,
tssecurity:S5334 - Change this code to not dynamically execute code influenced by user-controlled data. • BLOCKERView issue

Location: routes/showProductReviews.ts:34

Why is this an issue?

Code injections occur when applications allow the dynamic execution of code instructions from untrusted data.
An attacker can influence the behavior of the targeted application and modify it to get access to sensitive data.

What changed

This hunk fixes a code injection vulnerability in the MongoDB query. The original code used the dangerous $where operator with string concatenation ('this.product == ' + id), which allowed user-controlled input (id derived from the HTTP request) to be dynamically executed as JavaScript code on the MongoDB server. The fix replaces this with a standard MongoDB equality query ({ product: id }), which uses a safe operator that does not evaluate JavaScript expressions, thereby eliminating the possibility of code injection through the id parameter.

--- a/routes/showProductReviews.ts
+++ b/routes/showProductReviews.ts
@@ -34,1 +34,1 @@ module.exports = function productReviews () {
-    db.reviewsCollection.find({ $where: 'this.product == ' + id }).then((reviews: Review[]) => {
+    db.reviewsCollection.find({ product: id }).then((reviews: Review[]) => {
tssecurity:S3649 - Change this code to not construct SQL queries directly from user-controlled data. • BLOCKERView issue

Location: routes/login.ts:36

Why is this an issue?

Database injections (such as SQL injections) occur in an application when the application retrieves data from a user or a third-party service and inserts it into a database query without sanitizing it first.

What changed

This hunk fixes the SQL injection vulnerability in the login route. The original code directly interpolated user-controlled data (req.body.email) into the SQL query string using template literals, allowing an attacker to inject malicious SQL. The fix replaces the string interpolation with parameterized query using Sequelize's bind parameters ($email and $password), which ensures that user input is properly escaped and cannot alter the query logic. This is the standard prepared statement approach recommended for preventing SQL injection.

--- a/routes/login.ts
+++ b/routes/login.ts
@@ -36,1 +36,11 @@ module.exports = function login () {
-    models.sequelize.query(`SELECT * FROM Users WHERE email = '${req.body.email || ''}' AND password = '${security.hash(req.body.password || '')}' AND deletedAt IS NULL`, { model: UserModel, plain: true }) // vuln-code-snippet vuln-line loginAdminChallenge loginBenderChallenge loginJimChallenge
+    models.sequelize.query(
+      'SELECT * FROM Users WHERE email = $email AND password = $password AND deletedAt IS NULL',
+      {
+        bind: {
+          email: req.body.email || '',
+          password: security.hash(req.body.password || '')
+        },
+        model: UserModel,
+        plain: true
+      }
+    ) // vuln-code-snippet vuln-line loginAdminChallenge loginBenderChallenge loginJimChallenge
tssecurity:S2083 - Change this code to not construct the path from user-controlled data. • BLOCKERView issue

Location: routes/vulnCodeFixes.ts:80

Why is this an issue?

Path injections occur when an application uses untrusted data to construct a file path and access this file without validating its path first.

What changed

Adds the import for Node.js 'path' module, which is required by the path traversal fix in the other hunk. The path module provides path.resolve(), path.join(), path.basename(), and path.sep which are all used to implement canonical path validation and prevent the path injection vulnerability where user-controlled data (the 'key' parameter from the request body) is used to construct a file path.

--- a/routes/vulnCodeFixes.ts
+++ b/routes/vulnCodeFixes.ts
@@ -5,0 +6,1 @@ const fs = require('fs')
+const path = require('node:path')
tssecurity:S2083 - Change this code to not construct the path from user-controlled data. • BLOCKERView issue

Location: routes/vulnCodeSnippet.ts:94

Why is this an issue?

Path injections occur when an application uses untrusted data to construct a file path and access this file without validating its path first.

What changed

Adds the import for Node.js 'path' module, which is required by the path traversal fix in the other hunk. The path module provides path.resolve and path.sep used to implement canonical path validation that prevents the path injection vulnerability where user-controlled data (the 'key' parameter from the request body) was used to construct a file path without validation.

--- a/routes/vulnCodeSnippet.ts
+++ b/routes/vulnCodeSnippet.ts
@@ -8,0 +9,1 @@ import yaml from 'js-yaml'
+import path from 'node:path'

Have a suggestion or found an issue? Share your feedback here.


SonarQube Remediation Agent uses AI. Check for mistakes.

Fixed issues:
- AZWU-tnYYJSZqVQVbSkm for tssecurity:S3649 rule
- AZWU-tsdYJSZqVQVbSla for tssecurity:S5334 rule
- AZWU-tnOYJSZqVQVbSki for tssecurity:S5147 rule
- AZWU-trNYJSZqVQVbSlJ for tssecurity:S2083 rule
- AZWU-tnKYJSZqVQVbSka for tssecurity:S2083 rule

Generated by SonarQube Agent (task: cde90037-06be-4527-98ce-ac717a818e89)
Generated by SonarQube Agent (task: c79213b2-45d1-47d9-8886-643bf04fbeea)
@sonarqube-cloud-dev7

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant