fix lldpad netlink heap access overrun

[chunjiez]

If lldpad runs on top of some new versions of linux kernel (e.g. 6.10), then netlink message size would be larger than NLMSG_SIZE. In this case, lldpad application would crash due to heap access overrun.

[penguin359]

There should be no heap overrun or crash on the call to recv() because we pass the exact size of the allocated buffer. However, the parsing code of the netlink might have that possibility if they have embedded lengths and a truncated packet comes in. I'm not too familiar with how the netlink interface handles that case.

The logic generally seems sound with just a few comments added.

[chunjiez]

@penguin359 , could you please review the updated commit?

[penguin359]

I have finished my review and this seems reasonable. There is a possibility with the original code that if additional attributes are added to the message that the parsing code will overflow the default buffer size. From my own experiments with rtnetlink(2) this week-end, some netlink messages are indeed larger than 1024 bytes, but it seems none of the ones received by us so far.

It actually brings up a bigger issue with our use of the libnl API. We should be calling nlmsg_ok() on all received packets before any call to nlmsg_parse() as that does the bounds checking to make sure that the buffer has enough bytes in it to account for the total size of the message listed in it's header. If we had that check, then we would simply return an error with the original code instead of crashing due to out-of-bounds memory access.

Also, libnl does offer a function to handle much of the logic that is being added here in nl_recv() and friends, but are perhaps overkill with what they provide.

I see several other places in the code that seem to go into parsing netlink packets without a proper check that the advertised length is less than or equal to the buffer length for the packet. This includes a second, nearly identical use of nlmsg_parse() in this very same file. I approve this PR as is, but we should probably open an issue that tracks further improvements.

[apconole]

Can you please squash into a single commit @chunjiez

[chunjiez]

@apconole done

[apconole]

Thanks. Applied, and added a fixes tag.