La función parse_modbus_tcp_frame() lee el campo
length del encabezado MBAP directamente desde la red y lo usa sin validar como tamaño en
memcpy() hacia un buffer estático en el stack de 256 bytes.
Un atacante remoto puede enviar una trama Modbus con length=65535
provocando un desbordamiento clásico de stack con potencial de ejecución remota de código (RCE).
pdu_length <= 250 antes del memcpy, o usar buffer dinámico con
malloc(pdu_length) + comprobación de retorno. Compilar con -fstack-protector-strong
y habilitar PIE (-fPIE -pie) como mitigación defensiva en profundidad.
El hilo de heartbeat (pthread) mantiene un puntero a la estructura session_t
sin adquirir el mutex global antes de acceder a session->sock_fd.
Si el hilo principal llama a session_destroy() y libera la sesión
mientras el heartbeat está en tránsito, se produce un use-after-free con posibilidad de escritura controlada en heap.
atomic_fetch_add + atomic_fetch_sub) sobre session_t,
o adquirir session_mutex en el heartbeat antes de acceder a cualquier campo. Señalizar el hilo con
pthread_cancel + join antes de llamar a free().
La expresión topic_len * 2 + payload_len + 8 se calcula
con tipos uint16_t. Si topic_len ≥ 32764, el resultado desborda a un valor pequeño
y el malloc() reserva un buffer insuficiente. La escritura posterior de topic+payload provoca
heap overflow con datos controlados por el broker MQTT (o un broker comprometido).
size_t para la aritmética, o __builtin_add_overflow /
checked_add. Añadir assert(buf_sz < MQTT_MAX_PKT) y validar las
longitudes máximas de topic (MQTT spec: 65535 bytes, pero el SDK debería limitar mucho menos).
El mensaje de error recibido del cliente (campo error_msg en el handshake) se pasa directamente
como formato a fprintf(). Un atacante puede incluir especificadores de formato como
%n para escrituras arbitrarias en memoria o %s%s%s%s para lectura de stack.
fprintf(stderr, "%s", error_msg). Compilar con -Wformat-security -Werror
para detectar este patrón en el futuro.
La validación de tokens de sesión usa strcmp() en lugar de una comparación en tiempo constante.
Mediante mediciones de latencia de red (~100 ms por byte) es teóricamente posible deducir el token byte a byte.
El riesgo es bajo en redes con jitter pero presente en redes LAN industriales de baja latencia.
mbedtls_ct_memcmp() o CRYPTO_memcmp(). Verificar longitud antes de comparar
para evitar timing leaks adicionales.
session_alloc() llama a malloc(sizeof(session_t)) y desreferencia el puntero
inmediatamente sin comprobar si es NULL. Bajo presión de memoria (más de ~2.000 sesiones concurrentes o
ataque de agotamiento de recursos), el gateway crashea con SIGSEGV sin posibilidad de recuperación.
if (!s) { log_error("OOM"); return NULL; } tras cada malloc().
Implementar un pool de sesiones con límite configurable para prevenir el agotamiento.
"version": "2.1.0",
"runs": [{
"tool": {
"driver": {
"name": "c-review",
"version": "1.0.0",
"informationUri": "https://github.com/trailofbits/skills"
}
},
"results": [
{
"ruleId": "BUF-001",
"level": "error",
"message": { "text": "Stack buffer overflow in parse_modbus_tcp_frame() via unchecked MBAP length field" },
"locations": [{ "physicalLocation": { "artifactLocation": { "uri": "src/modbus_parser.c" }, "region": { "startLine": 187 } } }],
"properties": { "severity": "CRITICAL", "fp_verdict": "TRUE_POSITIVE", "attack_vector": "NETWORK" }
},
{
"ruleId": "UAF-001",
"level": "error",
"message": { "text": "Use-after-free in session_destroy() racing with heartbeat thread" },
"locations": [{ "physicalLocation": { "artifactLocation": { "uri": "src/session_manager.c" }, "region": { "startLine": 312 } } }],
"properties": { "severity": "CRITICAL", "fp_verdict": "TRUE_POSITIVE", "attack_vector": "NETWORK" }
},
/* … 9 resultados más … */
]
}]