Skip to content

Fix critical security vulnerabilities in routes: injection and redirection flaws#35

Open
sonarqube-agent[bot] wants to merge 1 commit into
masterfrom
remediate-master-20260608-010110-c74ac4c2
Open

Fix critical security vulnerabilities in routes: injection and redirection flaws#35
sonarqube-agent[bot] wants to merge 1 commit into
masterfrom
remediate-master-20260608-010110-c74ac4c2

Conversation

@sonarqube-agent

Copy link
Copy Markdown

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

Why these issues? All five issues were BLOCKER severity vulnerabilities representing critical attack vectors. They were selected based on severity classification, with each fix addressing a distinct injection or code execution vulnerability that could be exploited for data manipulation, unauthorized access, or malicious redirection.

This PR addresses 5 critical security vulnerabilities across multiple routes: NoSQL injection in product reviews and order history, SQL injection in search, open redirection, and code injection in order processing. These fixes prevent attackers from manipulating database queries, executing arbitrary code, and redirecting users to malicious sites 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/createProductReviews.ts:19

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 product reviews creation route. The original code passed user-controlled values (req.params.id, req.body.message, req.body.author) directly into a MongoDB insert() call without sanitization. An attacker could craft these inputs as objects (e.g., { $ne: "" }) instead of plain strings, potentially manipulating the database query. By calling .toString() on each of these values before they are inserted into the database query object, the code ensures that only plain string values are used, preventing attackers from injecting complex MongoDB query operators through user-controlled HTTP request parameters and body fields.

--- a/routes/createProductReviews.ts
+++ b/routes/createProductReviews.ts
@@ -20,3 +20,3 @@ module.exports = function productReviews () {
-      product: req.params.id,
-      message: req.body.message,
-      author: req.body.author,
+      product: req.params.id.toString(),
+      message: req.body.message.toString(),
+      author: req.body.author.toString(),
tssecurity:S5147 - Change this code to not construct database queries directly from user-controlled data. • BLOCKERView issue

Location: routes/orderHistory.ts:36

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 history route. The original code passed req.params.id directly into a MongoDB query (ordersCollection.update), allowing an attacker to craft a malicious HTTP request where req.params.id is a complex object (e.g., { $ne: "" }) instead of a plain string, potentially manipulating the query logic. By calling .toString() on req.params.id, the fix ensures that only a plain string value is used in the _id filter of the database query, preventing attackers from injecting NoSQL operators through user-controlled input.

--- a/routes/orderHistory.ts
+++ b/routes/orderHistory.ts
@@ -36,1 +36,1 @@ module.exports.toggleDeliveryStatus = function toggleDeliveryStatus () {
-    await ordersCollection.update({ _id: req.params.id }, { $set: { delivered: deliveryStatus, eta } })
+    await ordersCollection.update({ _id: req.params.id.toString() }, { $set: { delivered: deliveryStatus, eta } })
tssecurity:S5146 - Change this code to not perform redirects based on user-controlled data. • BLOCKERView issue

Location: routes/redirect.ts:19

Why is this an issue?

Open redirection occurs when an application uses user-controllable data to redirect users to a URL.

What changed

This hunk directly addresses the open redirection vulnerability by replacing the direct use of user-controlled toUrl in res.redirect() with a sanitized version. Instead of passing the raw user input to res.redirect(toUrl), it first passes toUrl through the sanitizeRedirectUrl function and redirects to the sanitized result. This prevents an attacker from crafting a malicious URL in the query parameter that would redirect users to an external attacker-controlled domain.

--- a/routes/redirect.ts
+++ b/routes/redirect.ts
@@ -19,1 +19,2 @@ module.exports = function performRedirect () {
-      res.redirect(toUrl)
+      const safeUrl = sanitizeRedirectUrl(toUrl)
+      res.redirect(safeUrl)
tssecurity:S3649 - Change this code to not construct SQL queries directly from user-controlled data. • BLOCKERView issue

Location: routes/search.ts:23

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 routes/search.ts by replacing direct string interpolation of user-controlled data (criteria) into the SQL query with a parameterized query using bind variables. The original code used template literals ('%${criteria}%') to embed the criteria value directly into the SQL string, allowing an attacker to manipulate the query logic. The fix uses a named bind parameter ($criteria) in the query string and passes the actual value through Sequelize's bind option ({ bind: { criteria: '%' + criteria + '%' } }), which ensures the user input is treated as a literal value rather than executable SQL, preventing SQL injection attacks.

--- a/routes/search.ts
+++ b/routes/search.ts
@@ -23,1 +23,4 @@ module.exports = function searchProducts () {
-    models.sequelize.query(`SELECT * FROM Products WHERE ((name LIKE '%${criteria}%' OR description LIKE '%${criteria}%') AND deletedAt IS NULL) ORDER BY name`) // vuln-code-snippet vuln-line unionSqlInjectionChallenge dbSchemaChallenge
+    models.sequelize.query(
+      'SELECT * FROM Products WHERE ((name LIKE $criteria OR description LIKE $criteria) AND deletedAt IS NULL) ORDER BY name', // vuln-code-snippet vuln-line unionSqlInjectionChallenge dbSchemaChallenge
+      { bind: { criteria: `%${criteria}%` } }
+    )
tssecurity:S5334 - Change this code to not dynamically execute code influenced by user-controlled data. • BLOCKERView issue

Location: routes/b2bOrder.ts:22

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

Removes the import of the 'vm' module, which was used to dynamically execute code via vm.runInContext(). This import is no longer needed because the fix replaces the dynamic code execution with safe JSON.parse(), eliminating the code injection vulnerability where user-controlled data (orderLinesData) was being passed into dynamically executed code.

--- a/routes/b2bOrder.ts
+++ b/routes/b2bOrder.ts
@@ -6,1 +5,0 @@
-import vm = require('vm')

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


SonarQube Remediation Agent uses AI. Check for mistakes.

Fixed issues:
- AZjSWOS8aBAs4lm2zztR for tssecurity:S5147 rule
- AZjSWOZqaBAs4lm2zztj for tssecurity:S5147 rule
- AZWU-tnbYJSZqVQVbSkr for tssecurity:S3649 rule
- AZjSWOVYaBAs4lm2zztX for tssecurity:S5334 rule
- AZWU-tnHYJSZqVQVbSkY for tssecurity:S5146 rule

Generated by SonarQube Agent (task: 18b84d7c-2db1-4257-a1f3-5d40ee74736f)
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