macnockserver.c can misplace terminating NULL

[pbug44]

The rule goes "never trust the network" and on macnockserver.c line 94, it says this:

nock->hood[nock->hoodLen] = '\0';

instead of using nock->hoodLen, it's better to calculate the hoodlen from the received packetlength, which is called recvlen. It doesn't overrun the buffer though because the buffer is 2048 bytes and it's impossible to do so.

nock->hoodLen is maximally 255 bytes (type uint8_t).

I'm trying to figure out more fallout to this, but nothing comes to my mind right now, the only thing that touch nock->hood after is log_trace("%s"), one would have to trace it back inside there then to see if there is a buffer overflow or anythign of the likes.

Good evening.
-peter

[pbug44]

I already figured this one out yesterday and said it. Pretend you get a 8 byte packet consisting just of version (1byte), len (1byte) and mac (6 bytes), and they screwed len very high (to about 255), then they don't have to guess the hood name because it is still the payload from the hood from the previous packet (someone elses packet). So basically it allows blind passage to someone who doesn't know what hood a host (or router?) is in.
The fix is 2 fold then, becuase this can be abused even with calculating the hoodlen from the recvlen (fix #1). It should also memset(&buf, 0, sizeof(buf)); ie. zeroize the buffer (make sure the compiler doesn't optimize this out, may have to do a for() loop) in order to prevent this blind hood spoofing. After that it should be safe, (knock on w^Whood, I wanted to say that since yesterday :-))