[PATCH] babel: Check TLV framing before dereferencing tlv->type
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- proto/babel/packets.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proto/babel/packets.c b/proto/babel/packets.c index d4ecf649..991c1520 100644 --- a/proto/babel/packets.c +++ b/proto/babel/packets.c @@ -1373,10 +1373,6 @@ babel_process_packet(struct babel_pkt_header *pkt, int len, (byte *)tlv < end; tlv = NEXT_TLV(tlv)) { - /* Ugly special case */ - if (tlv->type == BABEL_TLV_PAD1) - continue; - /* The end of the common TLV header */ pos = (byte *)tlv + sizeof(struct babel_tlv); if ((pos > end) || (pos + tlv->length > end)) @@ -1386,6 +1382,10 @@ babel_process_packet(struct babel_pkt_header *pkt, int len, break; } + /* Ugly special case */ + if (tlv->type == BABEL_TLV_PAD1) + continue; + msg = sl_alloc(p->msg_slab); res = babel_read_tlv(tlv, &msg->msg, &state); if (res == PARSE_SUCCESS) -- 2.18.0
On Tue, Jul 10, 2018 at 11:56:40PM +0200, Toke Høiland-Jørgensen wrote:
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Hi I think that the current position is correct and the patch is not - the follow-up code ('The end of the common TLV header') checks for full 2-byte TLV header, while BABEL_TLV_PAD1 is just 1-byte padding.
--- proto/babel/packets.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/proto/babel/packets.c b/proto/babel/packets.c index d4ecf649..991c1520 100644 --- a/proto/babel/packets.c +++ b/proto/babel/packets.c @@ -1373,10 +1373,6 @@ babel_process_packet(struct babel_pkt_header *pkt, int len, (byte *)tlv < end; tlv = NEXT_TLV(tlv)) { - /* Ugly special case */ - if (tlv->type == BABEL_TLV_PAD1) - continue; - /* The end of the common TLV header */ pos = (byte *)tlv + sizeof(struct babel_tlv); if ((pos > end) || (pos + tlv->length > end)) @@ -1386,6 +1382,10 @@ babel_process_packet(struct babel_pkt_header *pkt, int len, break; }
+ /* Ugly special case */ + if (tlv->type == BABEL_TLV_PAD1) + continue; + msg = sl_alloc(p->msg_slab); res = babel_read_tlv(tlv, &msg->msg, &state); if (res == PARSE_SUCCESS) -- 2.18.0
-- 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."
Ondrej Zajicek <santiago@crfreenet.org> writes:
On Tue, Jul 10, 2018 at 11:56:40PM +0200, Toke Høiland-Jørgensen wrote:
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Hi
I think that the current position is correct and the patch is not - the follow-up code ('The end of the common TLV header') checks for full 2-byte TLV header, while BABEL_TLV_PAD1 is just 1-byte padding.
Yeah, you're right; sorry for the noise. Guess I was seeing things after looking at too many bounds check constructs last night :) -Toke
participants (2)
-
Ondrej Zajicek -
Toke Høiland-Jørgensen