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

Awesome new feature! #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Awesome new feature! #22

wants to merge 1 commit into from

Conversation

asadeddin
Copy link

We created an awesome new feature that'll take us to the moon!

Copy link

corgea bot commented Mar 25, 2024

🐕 Corgea is suggesting a fix

Corgea would like fix:

  • CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') with a severity of High.
  • On file routes/search.ts
  • Severity: High

💬 Fix Explanation

The fix involves replacing the direct string interpolation in the SQL query with parameterized queries to prevent SQL injection.

  • The original code directly inserted the user input into the SQL query, which could lead to SQL injection if the user input contained SQL commands.
  • The fix replaces this direct string interpolation with a parameterized query, where the user input is treated as a parameter and not directly inserted into the query.
  • This ensures that the user input is always treated as a literal string and not as part of the SQL command, thus preventing SQL injection.

🪄 Fix suggestion

diff --git a/routes/search.ts b/routes/search.ts
index b3f2daa..2bbc1fd 100644
--- a/routes/search.ts
+++ b/routes/search.ts
@@ -21,7 +21,8 @@ module.exports = function searchProducts () {
     let criteria: any = req.query.q === 'undefined' ? '' : req.query.q ?? ''
     criteria = (criteria.length <= 200) ? criteria : criteria.substring(0, 200)
     
-    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 :search OR description LIKE :search) AND deletedAt IS NULL) ORDER BY name`, 
+    { replacements: { search: `%${criteria}%` }, type: models.sequelize.QueryTypes.SELECT }) 
       .then(([products]: any) => {
         const dataString = JSON.stringify(products)
         if (challengeUtils.notSolved(challenges.unionSqlInjectionChallenge)) { // vuln-code-snippet hide-start

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