Fix Buffer overflow in cupsSideChannelSNMPGet() - #1719 - #1720
PavlNekrasov wants to merge 1 commit into
Conversation
|
In the future, please combine bug report and PR into a single PR with the full explanation of the bug. Doing both just gives both of us extra work. |
|
Also, please start signing your commits... |
Problem: cupsSideChannelSNMPGet() takes the OID length from strlen() without checking that the nul terminator is inside the bytes the backend actually sent, so strlen() runs into the uninitialised tail of the buffer, real_datalen goes negative and memcpy() gets (size_t)real_datalen as its size. cupsSideChannelSNMPWalk() has the same flaw, and its guard against it measures sizeof() a char pointer instead of the buffer, so it only fires below 8 bytes. Solution: Located the OID terminator with memchr() within the received length in both functions and rejected a response without one as CUPS_SC_STATUS_BAD_MESSAGE. Signed-off-by: p.nekrasov@fobos-nt.ru Signed-off-by: Timofei Fedotov sovtouch@altlinux.org
c784ad3 to
a69f054
Compare
|
The commit has now been signed and forced-pushed. |
|
OK, so I am still having trouble accepting this PR. The maximum buffer length in cups/sidechannel.c is 64k+4 bytes: command byte, status byte, two length bytes (to allow data up to 65535 bytes), and 65536 bytes to allow for a 65535-byte string + nul. The data buffer in backend/network.c is 65536 bytes - 65535 bytes + nul. So the only issue here is that we aren't ensuring that the byte after the first nul in the response is part of the message/buffer. Using memchr doesn't actually help here, but what we want to know is that we actually have a value. |
2 participants
fixed #1719
Solution: Located the OID terminator with memchr() within the received length in both functions and rejected a response without one as CUPS_SC_STATUS_BAD_MESSAGE.
Signed-off-by: p.nekrasov@fobos-nt.ru
Signed-off-by: Timofei Fedotov sovtouch@altlinux.org