Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

emcute: never return from receive loop #12382

Merged
merged 1 commit into from Oct 7, 2019

Conversation

nmeum
Copy link
Member

@nmeum nmeum commented Oct 7, 2019

Contribution description

The receive thread of the MQTT-SN implementation emcute returns from the packet receive loop if sock_udp_recv returns an error. This is, for instance, the case when a given packet doesn't fit into the supplied buffer. This condition can be triggered by anyone able to send large packets (> 512 byte) to the UDP socket. If so, this will stop the MQTT client from working until the device is restarted, i.e. emcute_run is invoked again.

while (1) {
ssize_t len = sock_udp_recv(&sock, rbuf, sizeof(rbuf), t_out, &remote);
if ((len < 0) && (len != -ETIMEDOUT)) {
LOG_ERROR("[emcute] error while receiving UDP packet\n");
return;
}

Testing procedure

My tap configuration is as follows:

4: tap0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
    link/ether 32:e2:ba:2b:07:d1 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::30e2:baff:fe2b:7d1/64 scope link 
       valid_lft forever preferred_lft forever
  1. Enable debug in sys/net/application_layer/emcute/emcute.c.
  2. Compile and run examples/emcute_mqttsn.
  3. Connect to a bugos MQTT server address.
  4. Spoof a server reply with a packet exceeding the default buffer size (512 byte). This causes sock_udp_recv to return -ENOBUFS.
  5. Attempt to connect to a real working MQTT server (will fail).
MQTT-SN example application

Type 'help' to get started. Have a look at the README.md for more information.
> con fe80::30e2:baff:fe2b:7d1 2342

Spoof the reply, e.g. using:

head -c 525 < /dev/urandom  | busybox nc -p 2342 -u 'fe80::30e2:baff:fe2b:7d2%tap0' 1883

This should cause emcute_run to return and output the message [emcute] error while receiving UDP packet. Afterwards, it should no longer be possible to connect to a working MQTT-SN server since emcute will nerver receive the CONNACK reply from it since it doesn't read from the socket anymore.

Impact

Denial of service, a spoofed server message will cause the emcute client to stop working until emcute_run is invoked again which is usually the case when the device is restarted.

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`.
@benpicco benpicco added Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Oct 7, 2019
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change makes sense, the emcute server thread should not exit on client error.

@miri64
Copy link
Member

miri64 commented Oct 7, 2019

Will try to remember to add a test case for that to #11823.

@miri64
Copy link
Member

miri64 commented Oct 7, 2019

Ok, wasn't that hard...: edae2c2. I was able to confirm this bug fix using that patch and cherry-picking 74e19d4 and 10a3f3e on top (and without of course).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants