[PATCH] Babel: Fix pointer arithmetic in subtlv parsing
The subtlv parsing code was doing byte-based arithmetic with non-void pointers, causing it to read beyond the end of the packet. Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- proto/babel/packets.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/babel/packets.c b/proto/babel/packets.c index 1088fab7..269f04ef 100644 --- a/proto/babel/packets.c +++ b/proto/babel/packets.c @@ -951,7 +951,7 @@ babel_read_subtlvs(struct babel_tlv *hdr, struct babel_tlv *tlv; for (tlv = (void *) hdr + state->current_tlv_endpos; - tlv < hdr + TLV_LENGTH(hdr); + (void *) tlv < (void *) hdr + TLV_LENGTH(hdr); tlv = NEXT_TLV(tlv)) { /* -- 2.13.1
On Tue, Jun 13, 2017 at 11:01:11PM +0200, Toke Høiland-Jørgensen wrote:
The subtlv parsing code was doing byte-based arithmetic with non-void pointers, causing it to read beyond the end of the packet.
Thanks, merged. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Hello, world!\n
for (tlv = (void *) hdr + state->current_tlv_endpos; - tlv < hdr + TLV_LENGTH(hdr); + (void *) tlv < (void *) hdr + TLV_LENGTH(hdr); tlv = NEXT_TLV(tlv))
BTW, is there any reason for doing that pointer arithmetics on void pointers instead of char pointers? It looks like an unnecessary GCCism. Have a nice fortnight -- Martin `MJ' Mares <mj@ucw.cz> http://mj.ucw.cz/ Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth DRM 'manages access' in the same way that jail 'manages freedom.'
On Thu, Jun 15, 2017 at 08:38:16AM +0200, Martin Mares wrote:
Hello, world!\n
for (tlv = (void *) hdr + state->current_tlv_endpos; - tlv < hdr + TLV_LENGTH(hdr); + (void *) tlv < (void *) hdr + TLV_LENGTH(hdr); tlv = NEXT_TLV(tlv))
BTW, is there any reason for doing that pointer arithmetics on void pointers instead of char pointers? It looks like an unnecessary GCCism.
Hi Well, it is not only here but already on plenty of other places in BIRD code. I don't have a strong opinion about pointer arithmetics on void pointers. I generally avoid it when writing code but otherwise keep it. It is true that it is GCCism that is simply eliminable, but it is also supported by Clang and possibly others as it is a natural extension. In this particular case it does not help, but in other cases it avoids some uninformative double casts. Now, when i am looking at this particular case, i realized it could be replaced by simple: tlv < NEXT_TLV(hdr); -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (3)
-
Martin Mares -
Ondrej Zajicek -
Toke Høiland-Jørgensen