pppd cracked when a malformed PPPoE Discovery package recieved

[jovemars]

on the time pppd process recieves a malformed PPPoE Discovery package, such as the payload length value is greater then the frame size, or one of the tag length value is greater than the last payload length after this tag, the parsePackage function will be out of memory bounds. the process will crack because of SIGSEGV.

[nihilus]

@jovemars neat finding there, did you report it upstream to the roaring penguin folks?

[paulusmack]

jovemars: Is this an actual observation, or something you inferred by reading the code? If it's an actual observation and you have a reproducer, please send it to me privately so I can use it for checking the fix.

I have looked through the code and I'm not seeing the issue you describe. Each call to receivePacket() is followed by a check like this:

/* Check length */
if (ntohs(packet.length) + HDR_SIZE > len) {
    fprintf(stderr, "Bogus PPPoE length field (%u)\n",
	   (unsigned int) ntohs(packet.length));
    continue;
}

So we are in fact checking the payload length value against the received frame size. The semantics of the recv() function will ensure that we don't overflow the 'packet' variable.

Then in parsePacket, the loop that looks at the tags has this check in it:

if ((curTag - packet->payload) + tagLen + TAG_HDR_SIZE > len) {
    error("Invalid PPPoE tag length (%u)", tagLen);
    return -1;
}

So we are in fact checking the tag length value against the remaining payload size.

The condition for the while loop that iterates over the tags isn't perfect:

while(curTag - packet->payload < len) {

It should be "curTag - packet->payload + TAG_HDR_SIZE <= len". However, while that might lead to reading parts of packet->payload past the end of the received frame, the check on the tag length value will always fail, and I don't see any possibility of accessing past the end of the packet->payload array.

Thus I don't understand how we can get a SIGSEGV from receiving a malformed PPPoE frame, and I would appreciate a more specific example (privately to [email protected] if you don't want to reveal details publicly).

[Neustradamus]

@jovemars: Have you seen the reply of @paulusmack (30 Dec 2019)?

Please look your email.

[Neustradamus]

@jovemars: Have you seen the reply of @paulusmack (30 Dec 2019)?

[Neustradamus]

@jovemars: Have you seen the reply of @paulusmack (30 Dec 2019)?

Please look your email.

[dfskoll]

I do not think the situations described by the OP are exploitable; I would close this issue.

[dfskoll]

Though, I did make the change to the while loop condition suggested by @paulusmack in upstream https://github.com/dfskoll/rp-pppoe/commit/245557a1155897e7b90050f5e1680d9cec3399b0