T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/amap-proxy.js:9
- Finding
- Unauthenticated Shell Command Injection in the Amap Proxy<![CDATA[ ## Vulnerability Details **File Location**: `scripts/amap-proxy.js`, lines 9–25, 45–53, and 74–101 **Vulnerability Type**: OS command injection through unauthenticated HTTP parameters **Risk Level**: Critical ### Vulnerable Code ```javascript // CORS headers const corsHeaders = { 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, OPTIONS', 'Access-Control-Allow-Headers': 'Origin, X-Requested-With, Content-Type, Accept' }; // Handle search requests function handleSearch(query, city, response) { // Use relative path to amap-maps skill const amapMapsDir = path.join(__dirname, '..', '..', 'amap-maps'); // Use default API key or allow override via environment variable const amapKey = process.env.AMAP_KEY || "88628414733cf2ccb7ce2f94cfd680ef"; const command = `cd "${amapMapsDir}" && AMAP_KEY="${amapKey}" node scripts/amap.js search text "${query}" ${city}`; exec(command, { timeout: 10000 }, (error, stdout, stderr) => { if (error) { console.error('Search error:', error); response.writeHead(500, corsHeaders); response.end(JSON.stringify({ error: 'Search failed', details: error.message })); return; } // ... }); } function handleDetail(poiId, response) { const amapMapsDir = path.join(__dirname, '..', '..', 'amap-maps'); const amapKey = process.env.AMAP_KEY || "88628414733cf2ccb7ce2f94cfd680ef"; const command = `cd "${amapMapsDir}" && AMAP_KEY="${amapKey}" node scripts/amap.js search detail "${poiId}"`; exec(command, { timeout: 10000 }, (error, stdout, stderr) => { // ... }); } ``` The affected values originate from HTTP requests: ```javascript if (pathname === '/api/search' && req.method === 'GET') { const query = parsedUrl.query.q; const city = parsedUrl.query.city || '重庆'; // ... handleSearch(query, city, res); } if (pathname.startsWith('/api/detail ...[truncated 1974 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace `exec()` with `execFile()` or `spawn()` and pass every argument as a separate array element: ```javascript const { execFile } = require('child_process'); execFile( 'node', [ path.join(amapMapsDir, 'scripts', 'amap.js'), 'search', 'text', query, city ], { cwd: amapMapsDir, env: { ...process.env, AMAP_KEY: amapKey }, timeout: 10000 }, callback ); ``` 2. Validate request parameters before execution: - Enforce length limits. - Reject control characters. - Restrict city and POI identifiers to expected character sets and formats. 3. Bind the service explicitly to `127.0.0.1`. 4. Replace wildcard CORS with an explicit allowlist of trusted origins. 5. Add request authentication if the proxy can ever be exposed beyond loopback. 6. Apply request-rate limits and return generic errors without command details. 7. Never interpolate environment values into shell command strings. ]]>