Add login functionality for Juice Shop

[eduappsec1]

No description provided.

[github-actions]

🚫 [Bearer] <javascript_lang_sql_injection> _{reported by reviewdog 🐶}

Unsanitized input in SQL query

Description

Using unsanitized data, such as user input or request data, or externally influenced data passed to a function, in SQL query exposes your application to SQL injection attacks. This vulnerability arises when externally controlled data is directly included in SQL statements without proper sanitation, allowing attackers to manipulate queries and access or modify data.

Remediations

• Do not use raw SQL queries that concatenate unsanitized input directly.

  var sqlite = new Sequelize("sqlite::memory:");
  sqlite.query("SELECT * FROM users WHERE ID = " + req.params.userId); // unsafe

• Do validate all query inputs to ensure they meet expected patterns or values before using them in a query.

  var rawId = req.params.userId
  if !(/[0-9]+/.test(rawId)) {
    // input is unexpected; don't make the query
  }

• Do use prepared (or parameterized) statements for querying databases to safely include external input.

  var sqlite = new Sequelize("sqlite::memory:");
  sqlite.query(
    "SELECT * FROM users WHERE ID = ?",
    { replacements: [req.params.userId] },
    type: sequelize.QueryTypes.SELECT
  )

References

• OWASP SQL injection explained
• OWASP SQL injection prevention cheat sheet

[github-actions]

🚫 [Bearer] <javascript_lang_observable_timing> _{reported by reviewdog 🐶}

Observable Timing Discrepancy

Description

Observable Timing Discrepancy occurs when the time it takes for certain operations to complete can be measured and observed by attackers. This vulnerability is particularly concerning when operations involve sensitive information, such as password checks or secret comparisons. If attackers can analyze how long these operations take, they might be able to deduce confidential details, putting your data at risk.

Remediations

• Do implement algorithms that process sensitive information in constant time. This approach helps prevent attackers from guessing secrets based on the duration of operations.
• Do use built-in security features and cryptographic libraries that offer functions safe from timing attacks for comparing secret values.
• Do not use direct string comparisons for sensitive information, as this can lead to early termination of the function if a mismatch is found, revealing timing information.

    if (apiToken === "zDE9ET!TDq2uZx2oM!FD2") { // unsafe
      ...
    }

• Do not design application logic that changes execution paths in a manner that could introduce timing discrepancies based on user input or secret values.

References

• OWASP Guide to Cryptography
• MDN Web Docs on SubtleCrypto API

[aikido-pr-checks]

Potential SQL injection via string-based query concatenation - critical severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Suggested change

	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`, { replacements: { email: req.body.email || '', password: security.hash(req.body.password || '') }, model: UserModel, plain: true }) // vuln-code-snippet vuln-line loginAdminChallenge loginBenderChallenge loginJimChallenge

_{Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info}

[aikido-pr-checks]

Exposed secret in login-juice-shop.js - high severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

_{Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info}

[aikido-pr-checks]

🔵 Low - The new Juice Shop login flow is dead code in this application

As committed, this handler will never execute because the Express app still binds /login to routes.loginHandler and nothing imports login-juice-shop.js. Even if a later edit tried to wire it in, this file cannot load in the current codebase because the project is CommonJS and the referenced ../lib, ../data, and ../models trees do not exist here, so the PR does not actually add the login functionality it claims to introduce.

Show fix

Replace this transplanted Juice Shop module with a handler that matches this repository's runtime and data model, then explicitly mount it from app.js (or routes/index.js) and add a route-level test that exercises /login. If the new flow is not ready, remove this file from the PR to avoid a misleading dead-code feature addition.

_{More info - Reply on this comment to give feedback or ignore the issue.}