-
Notifications
You must be signed in to change notification settings - Fork 0
Test/sast autofix validation py #35
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import sqlite3 | ||
| from flask import Flask, request, jsonify | ||
|
|
||
| app = Flask(__name__) | ||
|
|
||
|
|
||
| def get_db(): | ||
| conn = sqlite3.connect("signatures.db") | ||
| conn.row_factory = sqlite3.Row | ||
| return conn | ||
|
|
||
|
|
||
| @app.route("/users") | ||
|
Check warning on line 13 in scripts/user_lookup.py
|
||
| def lookup_user(): | ||
| name = request.args.get("name", "") | ||
| role = request.args.get("role", "user") | ||
|
|
||
| conn = get_db() | ||
| cursor = conn.cursor() | ||
|
|
||
| query = "SELECT id, name, email, role FROM users WHERE name LIKE '%" + name + "%' AND role = '" + role + "'" | ||
| cursor.execute(query) | ||
|
Check failure on line 22 in scripts/user_lookup.py
|
||
|
Comment on lines
+21
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: SQL injection via string concatenation of The query at Line 21 concatenates Use the DB-API placeholder syntax ( 🔒 Proposed parameterized fix- query = "SELECT id, name, email, role FROM users WHERE name LIKE '%" + name + "%' AND role = '" + role + "'"
- cursor.execute(query)
+ query = "SELECT id, name, email, role FROM users WHERE name LIKE ? AND role = ?"
+ cursor.execute(query, (f"%{name}%", role))Additionally, if 🧰 Tools🪛 GitHub Check: SonarCloud Code Analysis[failure] 22-22: Change this code to not construct SQL queries directly from user-controlled data. 🪛 Ruff (0.15.12)[error] 21-21: Possible SQL injection vector through string-based query construction (S608) 🤖 Prompt for AI Agents |
||
|
|
||
| rows = cursor.fetchall() | ||
| conn.close() | ||
|
|
||
| return jsonify([dict(r) for r in rows]) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| app.run(host="0.0.0.0", port=5000) | ||
|
Check failure on line 31 in scripts/user_lookup.py
|
||
|
Comment on lines
+30
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid binding to
Default the bind address to 🛡️ Proposed change-if __name__ == "__main__":
- app.run(host="0.0.0.0", port=5000)
+if __name__ == "__main__":
+ import os
+ host = os.environ.get("APP_HOST", "127.0.0.1")
+ port = int(os.environ.get("APP_PORT", "5000"))
+ app.run(host=host, port=port)🧰 Tools🪛 ast-grep (0.42.1)[warning] 30-30: Running flask app with host 0.0.0.0 could expose the server publicly. (avoid_app_run_with_bad_host-python) 🪛 GitHub Check: SonarCloud Code Analysis[failure] 31-31: Avoid binding the application to all network interfaces. 🪛 Ruff (0.15.12)[error] 31-31: Possible binding to all interfaces (S104) 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,66 @@ | ||||||||||
| // file: userRoutes.js | ||||||||||
|
|
||||||||||
| // Line 10 - helper that builds the vulnerable query | ||||||||||
| function buildUserQuery(userId) { | ||||||||||
| // Vulnerable: string interpolation of untrusted input | ||||||||||
| return `SELECT * FROM users WHERE id = ${userId}`; | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential SQL injection via string-based query concatenation - critical severity Show fixRemediation: If possible, rebuild the query to use prepared statements or an ORM. Alternatively, use parseInt() to sanitize variables. If that is not possible, make sure the user input is verified or sanitized in another way. To autofix all SQL injection instances in your entire app, install Zen for Node.js. Reply There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AikidoSec ignore: this is a test reason There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Based on your feedback, we ignored this issue because of the following reason:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 We were not able to ignore this issue because of the following reason:
|
||||||||||
| } | ||||||||||
|
Comment on lines
+4
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Identify the DB driver in use to pick correct placeholder syntax.
fd -e json -g 'package.json' --exec cat {} \;
rg -nP -C2 '\b(pg|mysql2?|sqlite3|better-sqlite3|knex|pg-promise|sequelize|prisma)\b'
rg -nP -C3 '\bdb\s*=\s*'
rg -nP -C3 'registerUserRoutes\s*\('Repository: dejarno/signature-generator Length of output: 1574 🏁 Script executed: cat -n src/jarno.tsRepository: dejarno/signature-generator Length of output: 2384 Critical: SQL injection via direct interpolation of
Use parameterized queries / placeholders provided by your DB driver instead of string concatenation. The exact placeholder syntax depends on the driver ( 🔒 Proposed fix (pg-style placeholders shown; adjust to match your driver)-function buildUserQuery(userId) {
- // Vulnerable: string interpolation of untrusted input
- return `SELECT * FROM users WHERE id = ${userId}`;
- }
+function buildUserQuery() {
+ return 'SELECT * FROM users WHERE id = $1';
+ }- const query = buildUserQuery(userId);
- try {
- const result = await db.query(query);
+ try {
+ const result = await db.query(buildUserQuery(), [userId]);🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| // Line 16 - some unrelated utility code to push the fix further away | ||||||||||
| function logRequest(req) { | ||||||||||
| console.log(`[${new Date().toISOString()}] ${req.method} ${req.url}`); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| function checkAuth(req) { | ||||||||||
| // placeholder for auth logic | ||||||||||
| return !!req.headers['x-api-key']; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| function notFound(res) { | ||||||||||
| res.status(404).send('Not found'); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| function badRequest(res, message) { | ||||||||||
| res.status(400).send(message || 'Bad request'); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Many more unrelated helpers... | ||||||||||
| function trackMetric(name, value) { | ||||||||||
| console.log(`metric: ${name}=${value}`); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| function sanitizeOutput(user) { | ||||||||||
| // pretend this is doing something complex | ||||||||||
|
Comment on lines
+32
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment rephrases implementation ('pretend this is doing something complex') without explaining intent. Replace with why the sanitization is necessary or remove it.
Suggested change
Details✨ AI Reasoning Reply |
||||||||||
| const { password, ...safeUser } = user; | ||||||||||
| return safeUser; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Line ~50 – main Express route where SAST reports the issue | ||||||||||
| async function registerUserRoutes(app, db) { | ||||||||||
| app.get('/user', async (req, res) => { | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The anonymous async route handler passed to app.get is large and contains business logic; extract or give it a descriptive name and document it to improve readability and testability. Details✨ AI Reasoning 🔧 How do I fix it? Reply
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aikido ignore: no no no no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Based on your feedback, we ignored this issue because of the following reason:
|
||||||||||
| logRequest(req); | ||||||||||
| trackMetric('user_route_hit', 1); | ||||||||||
|
|
||||||||||
| const userId = req.query.userId; | ||||||||||
| if (!userId) { | ||||||||||
| return badRequest(res, 'Missing userId'); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Line ~65 - SAST flags data flow here (sink) | ||||||||||
| const query = buildUserQuery(userId); | ||||||||||
| try { | ||||||||||
| const result = await db.query(query); // SAST points to this call / this line | ||||||||||
| if (!result.rows[0]) { | ||||||||||
| return notFound(res); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const safeUser = sanitizeOutput(result.rows[0]); | ||||||||||
| res.json(safeUser); | ||||||||||
| } catch (err) { | ||||||||||
| console.error('DB error', err); | ||||||||||
| res.status(500).send('Internal server error'); | ||||||||||
| } | ||||||||||
| }); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| module.exports = { registerUserRoutes }; | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { NextRequest, NextResponse } from "next/server"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getPagesData } from "../../../utils/fetchingFunctions"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { DEFAULT_LOCALE } from "../../../utils/localization"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { draftMode } from "next/headers"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { redirect } from "next/navigation"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const PREVIEW_SECRET = "preview"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function GET(req: NextRequest) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const secret = req.nextUrl.searchParams.get("secret"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (secret !== PREVIEW_SECRET) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return NextResponse.json({ error: "Invalid token" }, { status: 401 }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A timing attack might allow hackers to bruteforce passwords - high severity Show fix
Suggested change
Reply There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AikidoSec ignore: this is a test reason number two There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 We were not able to ignore this issue because of the following reason:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 We were not able to ignore this issue because of the following reason:
Comment on lines
+7
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use an environment-backed preview secret instead of a hardcoded token.
Proposed fix-const PREVIEW_SECRET = "preview";
+const PREVIEW_SECRET = process.env.PREVIEW_SECRET;
export async function GET(req: NextRequest) {
const secret = req.nextUrl.searchParams.get("secret");
- if (secret !== PREVIEW_SECRET) {
+ if (!PREVIEW_SECRET) {
+ return NextResponse.json({ error: "Preview secret is not configured" }, { status: 500 });
+ }
+
+ if (!secret || secret !== PREVIEW_SECRET) {
return NextResponse.json({ error: "Invalid token" }, { status: 401 });
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const slugFromParams = req.nextUrl.searchParams.get("slug"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const slug = slugFromParams | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? Array.isArray(slugFromParams) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? slugFromParams[0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : slugFromParams | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check warning on line 20 in src/sast.ts
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+17
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nested ternary used to compute slug (ternary inside ternary). Break into clear if/else or separate assignments for readability. Show fix
Suggested change
Details✨ AI Reasoning Reply |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const localeFromParams = req.nextUrl.searchParams.get("locale"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const locale = localeFromParams | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? Array.isArray(localeFromParams) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? localeFromParams[0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : localeFromParams | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check warning on line 27 in src/sast.ts
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : DEFAULT_LOCALE; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const pageData = await getPagesData(slug, true, locale); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!pageData) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return NextResponse.json({ message: "Invalid slug" }, { status: 401 }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return a resource error status for invalid slug instead of 401.
Proposed fix- if (!pageData) {
- return NextResponse.json({ message: "Invalid slug" }, { status: 401 });
- }
+ if (!pageData) {
+ return NextResponse.json({ message: "Invalid slug" }, { status: 404 });
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| draftMode().enable(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirect(`/${locale}/${slug}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sanitize and validate redirect path segments before
Proposed fix const localeFromParams = req.nextUrl.searchParams.get("locale");
- const locale = localeFromParams
+ const locale = localeFromParams
? Array.isArray(localeFromParams)
? localeFromParams[0]
: localeFromParams
: DEFAULT_LOCALE;
+ const normalizedLocale = locale.replace(/^\/+|\/+$/g, "");
+ const normalizedSlug = slug.replace(/^\/+/, "");
+
+ // Prefer your existing locale allowlist/source of truth here.
+ const safeLocale = normalizedLocale || DEFAULT_LOCALE;
+
const pageData = await getPagesData(slug, true, locale);
if (!pageData) {
return NextResponse.json({ message: "Invalid slug" }, { status: 401 });
}
draftMode().enable();
- redirect(`/${locale}/${slug}`);
+ redirect(`/${safeLocale}/${normalizedSlug}`);
}📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: SonarCloud Code Analysis[warning] 25-27: Extract this nested ternary operation into an independent statement. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Show fix
Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is verified or sanitized. As an added layer of protection, we also recommend installing a WAF that blocks SQL injection attacks.
Reply
@AikidoSec ignore: [REASON]to ignore this issue.More info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aikido ignore