Se concatena directamente el userID proporcionado por el cliente en la query SQL. Un atacante puede manipular este parámetro para extraer o modificar datos de cualquier usuario de la plataforma.
func BuildUserQuery(userID string, dateRange string) string { // PELIGRO: concatenación directa — SQL Injection query := "SELECT * FROM metrics WHERE user_id = '" + userID + "' AND created_at >= '" + dateRange + "'" return query }
func GetUserMetrics(ctx context.Context, db *sql.DB, userID string, dateRange time.Time) ([]Metric, error) { // Parámetros preparados — nunca concatenar input externo rows, err := db.QueryContext(ctx, "SELECT id, value, created_at FROM metrics WHERE user_id = $1 AND created_at >= $2", userID, dateRange) if err != nil { return nil, fmt.Errorf("query user metrics: %w", err) } defer rows.Close() // ... }
El nombre de archivo generado incluye el userID sin sanitizar y se pasa directamente a exec.Command vía sh -c. Un atacante puede inyectar comandos arbitrarios en el sistema (; rm -rf /, exfiltración de datos, etc.).
func CompressReport(userID string, filePath string) error { // PELIGRO: sh -c con input no sanitizado cmd := exec.Command("sh", "-c", "zip report_"+userID+".zip "+filePath) return cmd.Run() }
func CompressReport(ctx context.Context, userID string, filePath string) error { // Validar que userID solo contiene caracteres seguros if !isValidUserID(userID) { return fmt.Errorf("invalid user id format") } // Argumentos como slice — nunca sh -c con input externo outputFile := filepath.Join("/tmp/reports", "report_"+userID+".zip") cmd := exec.CommandContext(ctx, "zip", outputFile, filePath) cmd.Stdin = nil return cmd.Run() }
El handler acepta un parámetro output_dir del cliente y lo usa directamente para escribir archivos sin comprobar que la ruta resultante esté dentro del directorio permitido. Permite escritura arbitraria en el sistema de archivos del servidor.
outputPath := r.URL.Query().Get("output_dir") filePath := filepath.Join(outputPath, userID+".csv") // filepath.Join no previene "../../../etc/passwd" f, err := os.Create(filePath)
const allowedBase = "/var/datapulse/exports" clean := filepath.Clean(filepath.Join(allowedBase, userID+".csv")) if !strings.HasPrefix(clean, allowedBase+"/") { http.Error(w, "forbidden path", http.StatusForbidden) return } f, err := os.Create(clean)
El error de rows.Close() y de file.Close() se descarta silenciosamente. Si la escritura en disco falla (espacio lleno, permisos) el cliente recibe un CSV corrupto sin ningún aviso.
_ = rows.Close() // error silenciado _ = file.Close() // error silenciado
if err := rows.Close(); err != nil { return fmt.Errorf("close rows: %w", err) } if err := file.Close(); err != nil { return fmt.Errorf("close export file: %w", err) }
Se lanza una goroutine para la compresión asíncrona sin ningún mecanismo de cancelación. Si el cliente desconecta o el servidor se reinicia, la goroutine queda activa indefinidamente consumiendo CPU y memoria.
go func() { CompressReport(userID, filePath) // sin contexto, sin WaitGroup }()
ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) defer cancel() if err := CompressReport(ctx, userID, filePath); err != nil { log.ErrorContext(ctx, "compress report", "error", err) http.Error(w, "internal server error", http.StatusInternalServerError) return }
12 instancias de return err sin envolver con fmt.Errorf("contexto: %w", err). En producción los logs muestran errores crípticos sin traza de qué operación falló, dificultando el debugging.
return err // ¿qué operación falló? imposible saberlo
return fmt.Errorf("write csv row for user %s: %w", userID, err)
El mutex se desbloquea manualmente al final de la función. Si se añade un return anticipado (por error o refactoring), el mutex queda bloqueado permanentemente causando un deadlock en todo el servicio de exportación.
mu.Lock() activeJobs[userID] = true // ... lógica con posibles returns anticipados ... mu.Unlock() // podría no ejecutarse nunca
mu.Lock() defer mu.Unlock() // siempre se ejecuta al salir de la función activeJobs[userID] = true
Se construye el CSV fila a fila concatenando strings (csvContent += row) en un bucle de potencialmente miles de iteraciones. Esto genera O(n²) allocations en heap. Para 10.000 filas, ~50x más lento que strings.Builder o csv.Writer.
Para cada fila de métricas se lanza una query adicional para obtener el nombre del evento. Con 1.000 filas = 1.001 roundtrips a PostgreSQL. Consolidar con un JOIN o un batch fetch.
ExportReportHandler mezcla validación, lógica de negocio, I/O y respuesta HTTP. Extraer en subfunciones: validateRequest, fetchMetrics, writeCSV, sendResponse.
func GenerateCSV(userID string, ctx context.Context, ...) — por convención Go, ctx siempre es el primer parámetro.
Ninguno de los 3 archivos nuevos tiene archivo _test.go correspondiente. Añadir tabla-driven tests para BuildUserQuery, CompressReport y los casos de validación de paths.
BuildUserQuery usando parámetros preparados ($1, $2)sh -c en CompressReport; pasar args como slice a exec.Commandfilepath.Clean + verificación de prefijo en el handler de exportaciónrows.Close() y file.Close()context.Context a CompressReport para evitar goroutine leakmu.Unlock() manual por defer mu.Unlock()csv.Writer (rendimiento)fmt.Errorf("ctx: %w", err) en las 12 instanciasExportReportHandler (87 líneas → subfunciones)ctx como primer parámetro en GenerateCSV