espressif / esp-mqtt

ESP32 mqtt component
Apache License 2.0
591 stars 254 forks source link

Is it safe to use 'strlen'? (IDFGH-8733) #241

Closed pherna06 closed 1 year ago

pherna06 commented 1 year ago

Been looking the repository code for a project and realized that, when publishing, topic is passed as a const char* to esp_mqtt_client_publish, but topic string length is not passed.

Eventually, that pointer gets to mqtt_msg_publish in mqtt_msg.c where this is done:

if (append_string(connection, topic, strlen(topic)) < 0) {
    return fail_message(connection);
}

My question is whether that use of 'strlen' is safe, as a pointer to an unsafe string (non null-terminated) could expose memory data in the topic of the publication.

euripedesrocha commented 1 year ago

Hi @pherna06, you are correct that this usage could be problematic, but it's a common pattern in C code to have NULL terminated char *. In this particular case, it's expected that the topic data is transmitted over a secure connection to a known broker in the user system, therefore some data exposure wouldn't be a great problem.

david-cermak commented 1 year ago

Please feel free to reopen