Skip to content

Test: Suggested Changes V3 #2

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.

Sign up for GitHub

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

Closed
Ashutosh0x wants to merge 4 commits into from
Closed

Test: Suggested Changes V3 #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
903837b
test: add more bugs
Ashutosh0x Jan 23, 2026
62b7273
test: add vulnerable file
Ashutosh0x Jan 23, 2026
a36c450
test: modify vulnerable file
Ashutosh0x Jan 23, 2026
f6cadb3
Add vulnerable test file
Ashutosh0x Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified mock-bug.js
View file
Open in desktop
Binary file not shown.
13 changes: 13 additions & 0 deletions vulnerable-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const sqlite3 = require('sqlite3').verbose();
const db = new sqlite3.Database(':memory:');

function getUser(id) {
const query = "SELECT * FROM users WHERE id = " + id;

@cubic-dev-ai cubic-dev-ai Bot Jan 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: SQL Injection Vulnerability: Building SQL queries via string concatenation allows attackers to inject malicious SQL. Use parameterized queries with placeholders instead.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At vulnerable-test.js, line 5:

<comment>**SQL Injection Vulnerability**: Building SQL queries via string concatenation allows attackers to inject malicious SQL. Use parameterized queries with placeholders instead.</comment>

<file context>
@@ -0,0 +1,13 @@
+const db = new sqlite3.Database(':memory:');
+
+function getUser(id) {
+    const query = "SELECT * FROM users WHERE id = " + id;
+    db.get(query, (err, row) => {
+        if (err) console.error(err);
</file context>
Fix with Cubic

db.get(query, (err, row) => {
if (err) console.error(err);

@Ashutosh0x Ashutosh0x Jan 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL Injection Vulnerability

Severity: CRITICAL | Confidence: 100%

Why: The getUser function in vulnerable.js is vulnerable to SQL injection, and this test case exploits it.

Suggested Fix:

Suggested change
if (err) console.error(err);
const query = "SELECT * FROM users WHERE id = ?";
db.get(query, [id], (err, row) => {

console.log(row);
});
Comment on lines +4 to +9

@coderabbitai coderabbitai Bot Jan 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "vulnerable-test.js" -type f

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 210

🏁 Script executed:

cd . && git ls-files | grep -E "(vulnerable|test)" | head -20

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 109

🏁 Script executed:

cat -n vulnerable-test.js

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 504

🌐 Web query:

sqlite3 node.js parameterized queries placeholders binding

💡 Result:

In Node.js, SQLite parameterized queries work by putting placeholders in your SQL, then binding values separately (array/object), so values are never string-interpolated into the SQL.

sqlite3 (npm sqlite3 / “node-sqlite3”) placeholders + binding

1) Anonymous (positional) placeholders: ?

Bind with an array (or positional arguments):

db.get("SELECT * FROM users WHERE id = ? AND active = ?", [id, 1], cb);

2) Indexed positional placeholders: ?1, ?2, …

SQLite indexes start at 1. You can bind with an array or numeric object keys. node-sqlite3 maps array indexes to start at 1 for these placeholders. [1]

db.run("UPDATE tbl SET name = ?5 WHERE id = ?", { 1: 2, 5: "bar" }, cb);

(Here ? is index 1, ?5 is index 5.) [1]

3) Named placeholders: $name, :name, @name

Bind with an object whose keys match the placeholder names:

db.run(
  "INSERT INTO test (id, value) VALUES ($id, $value)",
  { $id: 1, $value: "some value" },
  cb
);

If you pass extra/unused named keys, you can get SQLITE_RANGE: bind or column index out of range. [4]

Binding rule (important)

If you bind with an array or object, it must be the first bind argument; extra bind params after it are ignored. [1]

Related: built-in node:sqlite (newer Node)

Node’s built-in node:sqlite API explicitly binds parameters via separate namedParameters and anonymousParameters, and can optionally allow “bare” named keys (omit $/:/@ in your JS object) via setAllowBareNamedParameters(true). [2]

better-sqlite3 note

better-sqlite3 supports ? and named forms like @foo, :foo, $foo—but recommends not mixing named syntaxes within the same prepared statement. [3]

Sources: node-sqlite3 API wiki [1], Node node:sqlite docs [2], better-sqlite3 docs [3], StackOverflow example/error note [4].

Parameterize sqlite3 queries to prevent SQL injection.

Line 5 concatenates user input directly into the SQL string. Line 12 demonstrates how injection payloads like "1 OR 1=1" can alter query logic. Use sqlite3 parameterized queries with ? placeholders so values are treated as data, not executable SQL.

🔒 Proposed fix 
 function getUser(id) {
-    const query = "SELECT * FROM users WHERE id = " + id;
-    db.get(query, (err, row) => {
-        if (err) console.error(err);
-        console.log(row);
-    });
+    const query = "SELECT * FROM users WHERE id = ?";
+    db.get(query, [id], (err, row) => {
+        if (err) {
+            console.error(err);
+            return;
+        }
+        console.log(row);
+    });
 }
 
 const userId = "1 OR 1=1";
 getUser(userId);

Also applies to: 12-13

🤖 Prompt for AI Agents 
In `@vulnerable-test.js` around lines 4 - 9, The getUser function builds SQL by
concatenating id into query causing SQL injection; change db.get usage to use a
parameterized SQL statement (use "SELECT * FROM users WHERE id = ?" with
db.get's parameter array) and pass id as the parameter, keeping error handling
in the db.get callback (err and row) and logging/returning appropriately; update
any other places that build SQL similarly (e.g., other db.get/db.all calls) to
use ? placeholders instead of string concatenation.

}

const userId = "1 OR 1=1";

@Ashutosh0x Ashutosh0x Jan 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Demonstration of SQL Injection

Severity: CRITICAL | Confidence: 100%

Why: The userId is set to 1 OR 1=1, which, when passed to the vulnerable getUser function in vulnerable.js, results in a SQL query that bypasses the intended filtering and returns all rows from the users table.

Suggested Fix:

Suggested change
const userId = "1 OR 1=1";
const userId = "1";
getUser(userId);

getUser(userId);
6 changes: 6 additions & 0 deletions vulnerable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
function getUser(id) {
// TODO: Fix injection
const query = `SELECT * FROM users WHERE id = ${id}`;

@cubic-dev-ai cubic-dev-ai Bot Jan 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: SQL Injection Vulnerability: The id parameter is directly interpolated into the SQL query without sanitization. An attacker can inject malicious SQL (e.g., 1 OR 1=1 or 1; DROP TABLE users;--).

Use parameterized queries with placeholders instead of string interpolation. With sqlite3, use db.get("SELECT * FROM users WHERE id = ?", [id], callback) to safely bind the parameter.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At vulnerable.js, line 3:

<comment>**SQL Injection Vulnerability**: The `id` parameter is directly interpolated into the SQL query without sanitization. An attacker can inject malicious SQL (e.g., `1 OR 1=1` or `1; DROP TABLE users;--`).

Use parameterized queries with placeholders instead of string interpolation. With sqlite3, use `db.get("SELECT * FROM users WHERE id = ?", [id], callback)` to safely bind the parameter.</comment>

<file context>
@@ -0,0 +1,6 @@
+function getUser(id) {
+    // TODO: Fix injection
+    const query = `SELECT * FROM users WHERE id = ${id}`;
+    return query;
+}
</file context>
Fix with Cubic

@Ashutosh0x Ashutosh0x Jan 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL Injection Vulnerability

Severity: CRITICAL | Confidence: 100%

Why: The getUser function directly embeds the id parameter into the SQL query without any sanitization or parameterization. This allows an attacker to inject arbitrary SQL code by manipulating the id parameter.

Suggested Fix:

Suggested change
const query = `SELECT * FROM users WHERE id = ${id}`;
const query = `SELECT * FROM users WHERE id = ?`;
return query;

return query;
Comment on lines +1 to +4

@coderabbitai coderabbitai Bot Jan 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "vulnerable.js" -type f

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 92

🏁 Script executed:

cat -n ./vulnerable.js

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 281

🏁 Script executed:

rg "getUser" -B 2 -A 2

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 695

🏁 Script executed:

cat -n ./vulnerable-test.js

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 504

🏁 Script executed:

rg "require.*vulnerable" -A 3

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 59

🏁 Script executed:

find . -type f \( -name "*.js" -o -name "*.json" \) | head -20

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 254

🏁 Script executed:

grep -r "vulnerable" . --include="*.js" --include="*.json"

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 59

🏁 Script executed:

cat -n ./vulnerable.js

Repository: Ashutosh0x/gemini-code-review-bot

Length of output: 281

Block SQL injection by parameterizing the query.

Line 3 interpolates id directly into SQL, which is a critical injection flaw. Return a parameterized statement instead of a raw string.

🔒 Proposed fix 
 function getUser(id) {
     // TODO: Fix injection
-    const query = `SELECT * FROM users WHERE id = ${id}`;
-    return query;
+    return {
+        sql: 'SELECT * FROM users WHERE id = ?',
+        params: [id],
+    };
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function getUser(id) {
// TODO: Fix injection
const query = `SELECT * FROM users WHERE id = ${id}`;
return query;
function getUser(id) {
// TODO: Fix injection
return {
sql: 'SELECT * FROM users WHERE id = ?',
params: [id],
};
}
🤖 Prompt for AI Agents 
In `@vulnerable.js` around lines 1 - 4, The getUser function currently builds a
raw SQL string by interpolating id, which allows SQL injection; change getUser
to return a parameterized query instead (e.g., return an object with a query
text containing a placeholder and a values/params array) so the id is passed as
a bound parameter; update the function named getUser to stop string
interpolation of id and use a placeholder pattern your DB library expects (e.g.,
? or $1) and include id in the parameters collection (e.g., values or params) so
callers can execute the prepared statement safely.

@Ashutosh0x Ashutosh0x Jan 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL Injection Vulnerability

Severity: CRITICAL | Confidence: 100%

Why: The getUser function in vulnerable.js constructs a SQL query using template literals, directly embedding the user-provided id without any sanitization. This allows an attacker to inject arbitrary SQL code. The vulnerable-test.js file demonstrates this vulnerability.

Suggested Fix:

Suggested change
return query;
const query = `SELECT * FROM users WHERE id = ?`;
return query;

}
module.exports = { getUser };
Loading