Without this change an attacker would be able to stop the emcute server
by sending a crafted packet triggering this branch. The solution is
using `continue` instead of `return`.
This fixes a denial of service where an attacker would be able to cause
a NULL pointer dereference by sending a spoofed packet. This attack only
requires knowledge about pending message ids.
When a keepalive timeout occurs keepalive_retry_cnt remains zero,
so when the connection is re-established _on_keepalive_evt will
immediately disconnect instead of actually sending a keepalive ping.
The sequence looks like:
1. _on_connack: start con->keepalive_timer
2. Server does not respond to keepalive pings
3. _on_keepalive_evt: con->keepalive_retry_cnt reaches zero
4. Connection torn down and ASYMCUTE_DISCONNECTED sent to application
5. Application starts reconnection
6. _on_connack: start con->keepalive_timer again
7. First _on_keepalive_evt: con->keepalive_retry_cnt is still zero
8. Repeat from 4.
So this simply resets keepalive_retry_cnt in _on_connack when
the keepalive timer is restarted. It's a new connection, so
resetting the keepalive retry counter make senses regardless.
Signed-off-by: Derek Hageman <hageman@inthat.cloud>
The length field in an MQTT packet carries the _total_ length of the
packet. If it is below 256 (i.e. fits in one byte) only one byte is
used for the length field. If it is larger than that 3 bytes are used,
with the first byte having the value `0x01` and the remaining bytes
representing the length in as a 2 byte unsigned integer in network byte
order. Resulting from that it can be assessed that the check in
`emcutes`'s `set_len()` function is wrong as it needs to be checked if
`len` is lesser or equal to `0xff - 1`. `len <= (0xff - 1)` can be
simplified to `len < 0xff`. For some larger packages this safes 2 bytes
of wasted packet space.
`len` is used with the `memcpy()` to copy the payload to `tbuf`. With a
payload provided that is just long enough to fill `tbuf`, `len += 6`
leads to the `memcpy()` overriding data after `tbuf` (e.g. the
`mutex` that is unlocked right after) and thus resulting in potential
segmentation faults.
Additionally `+ 6` can only be applied if the total packet length is
below 256 (see spec), so `len + pos` is what needs to be provided to the
corresponding send functions instead (`pos` adapts to the header length
of the PUBLISH message).
