🛡️ Sentinel: [CRITICAL] Fix command injection and DoS in docker/forge logs

.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.

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;
         }

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.";
     }