halfgaar / FlashMQ

FlashMQ is a fast light-weight MQTT broker/server, designed to take good advantage of multi-CPU environments
https://www.flashmq.org/
Open Software License 3.0
173 stars 24 forks source link

Several protocol violations or bugs in FlashMQ #103

Closed songxpu closed 1 month ago

songxpu commented 1 month ago

Hi, I have found some protocol specification violations in my testing of FlashMQ, attached below are the details. The version of FlashMQ under tested: 1.11.0.

songxpu commented 1 month ago

According to the specification of MQTTv5.0:

[MQTT-3.1.2-13] 
If the Will Flag is set to 0, then Will Retain MUST be set to 0.

Replay packet The packet is a Connect (Will Flag is set to 0, but Will Retain is set to 1).

echo 10b90100044d51545405224bb42a11d243220f1600107777616a77486566557432765444657219002257e92600036a737a00054876754f79001d3876764b773344684f53737863723930624b737162516e796964734e461a010102388c5976080010563076736f675a4b4d54507945766a79000f3958786e9992dc4f4d51657a6934580014576b3545396f4a6e7a783263686537766844426400164144785748466d58587a33744a70313448356439445900094f57487a727a486d51  | xxd -p -r | nc 172.17.0.5 1883

image

Unexpected behavior: FlashMQ received such a packet and returned the Connect Ack packet with Success Code.

image

songxpu commented 1 month ago

Both the specification of MQTTv5.0 and MQTTv3.1.1 define:

[MQTT-3.1.2-9]
If the Will Flag is set to 1, the Will QoS and Will Retain fields in the Connect Flags will be used by the Server, and the Will Properties, Will Topic and Will Message fields MUST be present in the Payload.

However,

If we send a Connect packet that contains the will message property, but the will topic in it is empty, FlashMQ parses such a packet successfully and returns Connect ACK with a successful return code.

echo 105a00044d51545405f4003c032100140010636c69656e747878787878787878787800000000217878787878787878787878787878787878787878787878787878787878787878780007787878787878780009787878787878787878 | xxd -p -r | nc 127.0.0.1 1883

image

Furthermore, I have found that FlashMQ also saves such empty topic will messages to the database. This will affect the effectiveness of FlashMQ functionality on one hand, for example, when we restart FlashMQ, it will fail because the database contains publishing messages with empty topics:

image image

On the other hand, this may potentially lead to unexpected growth in database size, as each client with a client ID can send such empty topic will messages and save them.

halfgaar commented 1 month ago

Good finds :+1:

I also fixed it with will QoS which must be 0 when there is no will.

Fixed in master. I'll go ahead and close the ticket already, despite not being released yet. That will happen soon; there was going to be a release soon already.

songxpu commented 1 week ago

Hi @halfgaar, I found some potential security vulnerabilities in FlashMQ. Should I report them by submitting a Github issue? Or by email to discuss them

halfgaar commented 1 week ago

@songxpu Let's do e-mail first.