diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..c8c87cec --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-09 - Command Injection in docker logs via execSync +**Vulnerability:** Command injection and event loop blocking in `src/pages/api/docker-logs.ts` and `src/pages/api/forge-logs.ts` due to string concatenation with user input inside `execSync()`. +**Learning:** Using `execSync(command)` with user-derived inputs (like `tail` and `containerId`) allows arbitrary command execution if an attacker uses shell metacharacters like `;` or `&&`. Furthermore, synchronous execution blocks the main thread. +**Prevention:** Always use `execFile` or `spawn` with an array of arguments for OS commands, which prevents the shell from interpreting metacharacters. Additionally, validate inputs (e.g., rejecting strings starting with `-`) to prevent argument injection. diff --git a/src/pages/api/docker-logs.ts b/src/pages/api/docker-logs.ts index cb72e09e..5d86d87a 100644 --- a/src/pages/api/docker-logs.ts +++ b/src/pages/api/docker-logs.ts @@ -1,5 +1,8 @@ import type { APIRoute } from 'astro'; -import { execSync } from 'child_process'; +import { execFile } from 'child_process'; +import { promisify } from 'util'; + +const execFileAsync = promisify(execFile); export const GET: APIRoute = async ({ url }) => { try { @@ -14,23 +17,26 @@ export const GET: APIRoute = async ({ url }) => { const containerId = url.searchParams.get('id'); const tail = url.searchParams.get('tail') || '100'; - if (!containerId) { - return new Response(JSON.stringify({ error: "ID du conteneur manquant" }), { + // Validation de sécurité : éviter l'injection de drapeaux ou commandes + if (!containerId || containerId.startsWith('-') || tail.startsWith('-')) { + return new Response(JSON.stringify({ error: "ID du conteneur manquant ou invalide" }), { status: 400, headers: { 'Content-Type': 'application/json' } }); } - // Commande Docker pour récupérer les logs - const command = `docker logs --tail ${tail} ${containerId}`; - let logs = []; + let logs: string[] = []; try { - const output = execSync(command, { stdio: ['pipe', 'pipe', 'pipe'] }).toString(); - logs = output.trim().split('\n'); + // Utiliser execFile au lieu de execSync pour éviter les injections de commande et le blocage de l'event loop + const { stdout, stderr } = await execFileAsync('docker', ['logs', '--tail', tail, containerId]); + + // docker logs écrit souvent sur stderr (ou stdout + stderr combinés) + const combined = (stdout + '\n' + stderr).trim(); + logs = combined ? combined.split('\n') : []; } catch (err: any) { - // Certains logs sortent sur stderr, checkons stderr si stdout est vide ou si erreur - if (err.stderr) { - logs = err.stderr.toString().trim().split('\n'); + if (err.stderr || err.stdout) { + const combined = ((err.stdout || '') + '\n' + (err.stderr || '')).trim(); + logs = combined ? combined.split('\n') : []; } else { throw err; } diff --git a/src/pages/api/forge-logs.ts b/src/pages/api/forge-logs.ts index 68638bee..38435401 100644 --- a/src/pages/api/forge-logs.ts +++ b/src/pages/api/forge-logs.ts @@ -1,6 +1,9 @@ import type { APIRoute } from 'astro'; -import { execSync } from 'child_process'; +import { execFile } from 'child_process'; +import { promisify } from 'util'; import path from 'path'; + +const execFileAsync = promisify(execFile); export const GET: APIRoute = async ({ url }) => { try { @@ -12,7 +15,8 @@ export const GET: APIRoute = async ({ url }) => { let output = ""; try { - output = execSync(`tail -n ${lines} ${filePath}`).toString(); + const { stdout } = await execFileAsync('tail', ['-n', lines.toString(), filePath]); + output = stdout; } catch (e) { output = "Erreur lors de la lecture du fichier log."; }