martin-ger / esp_mqtt

MQTT Broker/Bridge on the ESP8266
MIT License
295 stars 69 forks source link

MQTT: Out of mem #15

Closed miolion closed 6 years ago

miolion commented 6 years ago

Hi,

I think this is very useful project for small IoT devices. I have found a minor issue if a client give NULL password to the broker...

For examble,

mosquitto_sub -h 192.168.4.1 -d -u "user" -P "" -t hello mosquitto_pub -h 192.168.4.1 -u "user" -P "" -t hello -m message

There are always "MQTT: Out of mem" in the debug log due to the following conditions in mqtt_server.c at line number 532.

if (password != NULL) clientcon->connect_info.password = (char *)os_zalloc(password_len+1); if (clientcon->connect_info.password != NULL) { ...

In that case, the value of 'clientcon->connect_info.password' will be always NULL.

SJ

martin-ger commented 6 years ago

Thanks for the report! This we just have to skip the "if (password != NULL)". Think it left over in the code from former editing.

miolion commented 6 years ago

Hi Martin,

No, I have found out that there will be a reset problem if an empty password is given from clients as follows,

mosquitto_pub -h 192.168.4.1 -t hello -u "none" -P "" -m message

So, it could be like this,

    if (password == NULL) {
        MQTT_WARNING("MQTT: Password invalid\r\n");
        MQTT_server_disconnectClientCon(clientcon);
        return;
    }

    clientcon->connect_info.password = (char *)os_zalloc(password_len+1);

The final modification decision will be up to you,

SJ

miolion commented 6 years ago

I have found out that Martin has resolved this issue.