Revisión de Código C++ — SensorHub IoT

📄 src/data_logger.cpp

5 CRÍTICOS 1 ALTO

CRÍTICO

Raw new/delete sin RAII L7–L11

El buffer se gestiona con new char[1024] manualmente. Si se lanza una excepción entre el constructor y el destructor, habrá memory leak. Viola RAII.

// ❌ ANTES
buffer_ = new char[1024];
...
delete[] buffer_;

✅ Corrección

// ✅ DESPUÉS
std::unique_ptr<char[]> buffer_ = std::make_unique<char[]>(1024);
// El destructor se elimina; unique_ptr libera automáticamente.

CRÍTICO

Null dereference en destructor L13–L16

fclose(file_) se llama sin verificar si fopen retornó nullptr. En sistemas IoT con disco lleno esto ocurre con frecuencia, causando crash por undefined behavior.

// ❌ file_ puede ser nullptr si fopen falla
fclose(file_);  // UB

✅ Corrección

if (file_) { fclose(file_); file_ = nullptr; }

CRÍTICO

Buffer overflow — strcpy sin bounds L19–L21

strcpy(buffer_, name) con un buffer de 1024 bytes no tiene protección. El nombre de dispositivo pasado en main supera los 50 caracteres, y en producción puede venir de red (>1 KB).

strcpy(buffer_, name); // ❌ sin límite

✅ Corrección

std::string device_name_;  // reemplaza char buffer_
setDeviceName(std::string_view name) {
    device_name_.assign(name.substr(0, 255)); // truncar con límite explícito
}

CRÍTICO

Format string attack L24–L26

printf(msg) y fprintf(file_, msg) con input controlado por el usuario permiten ataques de format string: lectura/escritura arbitraria de memoria. CWE-134.

printf(msg);          // ❌ format string attack
fprintf(file_, msg);  // ❌ ídem

✅ Corrección

printf("%s\n", msg);        // ✅ formato literal
fprintf(file_, "%s\n", msg); // ✅

ALTO

Mutex manual — riesgo de deadlock en excepción L30–L36

Si fwrite lanza una excepción (o se añade código entre lock/unlock), el mutex queda bloqueado permanentemente. Usar std::lock_guard con std::mutex es la solución RAII.

pthread_mutex_lock(&g_mutex);   // ❌ no exception-safe
...
pthread_mutex_unlock(&g_mutex);

✅ Corrección

std::mutex mutex_;  // miembro de clase
std::lock_guard<std::mutex> lock(mutex_); // ✅ RAII
file_.write(device_name_);

📄 src/sensor_reader.hpp

3 CRÍTICOS 2 ALTOS 1 MEDIO

CRÍTICO

malloc en C++ — memory leak + type-unsafe L8

malloc no llama constructores, no es type-safe y su pareja free no aparece en ningún lado. Combinar con delete es undefined behavior.

raw_data_ = (float*)malloc(256 * sizeof(float)); // ❌

✅ Corrección

std::vector<float> raw_data_; // en header
raw_data_.resize(256, 0.0f);   // ✅ RAII + inicializado

CRÍTICO

Thread detached — destructor vacío causa terminación abrupta L9, L14

El destructor está vacío: no hace join() ni detach(). Destruir un std::thread joinable sin join/detach llama a std::terminate(). El nodo IoT crashea.

~SensorReader() {} // ❌ std::terminate en tiempo de ejecución

✅ Corrección

~SensorReader() {
    running_.store(false);
    if (worker_ && worker_->joinable()) worker_->join();
}

CRÍTICO

Data race en variable running_ L22, L27

running_ es un bool normal leído desde el hilo worker y escrito desde el hilo principal sin sincronización. Comportamiento indefinido según el memory model de C++11.

bool running_ = true; // ❌ data race
while (running_) { ... }

✅ Corrección

std::atomic<bool> running_{true}; // ✅ lock-free
while (running_.load()) { ... }

ALTO

Violación de Rule of Five L5–L15

La clase gestiona recursos (puntero crudo + thread) pero no define copy-constructor, copy-assignment, move-constructor ni move-assignment. Cualquier copia accidental causará double-free o data races.

✅ Corrección mínima

// En la sección public:
SensorReader(const SensorReader&) = delete;
SensorReader& operator=(const SensorReader&) = delete;
SensorReader(SensorReader&&) = default;
SensorReader& operator=(SensorReader&&) = default;

MEDIO

Exposición de puntero crudo — getData() L17

Retornar un float* al interior del buffer rompe la encapsulación y permite al llamador escribir fuera de límites o retener un puntero inválido.

✅ Corrección

std::span<const float> getData() const noexcept {
    return {raw_data_.data(), raw_data_.size()};
}

📄 src/main.cpp

1 CRÍTICO 1 ALTO 1 MEDIO

CRÍTICO

Command injection — argv[2] sin sanitizar en system() L10–L12

El argumento argv[2] se interpola directamente en un comando de shell via sprintf + system(). Un atacante puede pasar "; rm -rf /" para ejecutar código arbitrario en el nodo IoT. CWE-78.

sprintf(cmd, "echo ... %s ...", argv[2]); // ❌
system(cmd);                                   // ❌ command injection

✅ Corrección — eliminar system() completamente

// Escribir directamente al log sin shell:
std::ofstream log("/var/log/sensores.log", std::ios::app);
log << "Iniciando sensor " << sanitize(argv[2]) << "\n";

ALTO

Acceso a argv[1]/argv[2] sin verificar argc L8, L10

Si el proceso se lanza sin argumentos (o con solo uno), argv[1] o argv[2] es nullptr. El crash en producción dejaría el nodo sin logging.

✅ Corrección

if (argc < 3) {
    std::cerr << "Uso: sensord <log_path> <device_id>\n";
    return EXIT_FAILURE;
}

MEDIO

Memory leak — SensorReader* nunca liberado L14

new SensorReader(42) nunca se libera. En un daemon de larga duración, las reinicios parciales acumulan leaks.

✅ Corrección

auto reader = std::make_unique<SensorReader>(42); // ✅

Criterios para aprobación final del PR #47

❌

Seguridad de memoria — Sin new/delete crudos

Pendiente: data_logger.cpp L7, sensor_reader.hpp L8-L9

❌

Seguridad — Sin command injection ni format string attacks

Pendiente: main.cpp L12 (system), data_logger.cpp L24-L25 (printf)

❌

Concurrencia — Variables atómicas o lock_guard

Pendiente: sensor_reader.hpp running_ no atómica; data_logger.cpp mutex manual

❌

RAII — Recursos ligados a ciclo de vida de objeto

Pendiente: FILE* sin RAII, threads sin join

❌

Rule of Five — Clases con recursos definen todos los especiales

Pendiente: SensorReader sin copy/move definidos o eliminados

⏳

Ejecutar clang-tidy y cppcheck sin warnings CRITICAL/HIGH

Pendiente tras aplicar correcciones: clang-tidy --checks='*,-llvmlibc-*' src/*.cpp -- -std=c++17

Informe de Revisión de Código C++

Criterios para aprobación final del PR #47