[lonetix/*,tools/bgpgrep/*] Correct handling of extended timestamps during filtering, decode and dump

lonetix/bgp/dump_isolario.c

@@ -153,6 +153,12 @@ static const char *Bgp_KnownCommunityToString(BgpCommCode code)
 	}
 }
 
+static void NormalizeExtendedTimestamp(Dumpfmtctx *ctx)
+{
+	ctx->timestamp += ctx->microsecs / 1000000;
+	ctx->microsecs %= 1000000;
+}
+
 static void DumpUnknown(Stmbuf *sb, BgpType type)
 {
 	Bufio_Putc(sb, UNKNOWN_MARKER);
@@ -420,7 +426,7 @@ static void DumpMrtInfoTrailer(Stmbuf *sb, const Dumpfmtctx *ctx)
 		Bufio_Putu(sb, ctx->timestamp);
 		if (ctx->microsecs > 0) {
 			Utoa(ctx->microsecs, buf);
-			Df_strpadl(buf, '0', 9);
+			Df_strpadl(buf, '0', 6);
 
 			Bufio_Putc(sb, '.');
 			Bufio_Puts(sb, buf);
@@ -941,14 +947,25 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr    *hdr,
 	ctx.isAsn32bit   = BGP4MP_ISASN32BIT(hdr->subtype);
 	ctx.isAddPath    = BGP4MP_ISADDPATH(hdr->subtype);
 	ctx.timestamp    = beswap32(hdr->timestamp);
-	if (hdr->type == MRT_BGP4MP_ET)
+	if (hdr->type == MRT_BGP4MP_ET) {
+		// Length must account for extended timestamp
+		if (len < 4) goto corrupt;
+
 		ctx.microsecs = beswap32(((const Mrthdrex *) hdr)->microsecs);
+		len -= 4;
+
+		NormalizeExtendedTimestamp(&ctx);
+	}
 
 	ctx.hdr = hdr;
 
 	if (ctx.isAsn32bit) {
+		if (len < sizeof(bgp4mp->a32)) goto corrupt;
+
 		ctx.peerAs = beswap32(bgp4mp->a32.peerAs);
 		if (bgp4mp->a32.afi == AFI_IP) {
+			if (len < sizeof(bgp4mp->a32v4)) goto corrupt;
+
 			ctx.peerAddr.family = IP4;
 			ctx.peerAddr.v4     = bgp4mp->a32v4.peerAddr;
 
@@ -956,14 +973,20 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr    *hdr,
 		} else {
 			assert(bgp4mp->a32.afi == AFI_IP6);
 
+			if (len < sizeof(bgp4mp->a32v6)) goto corrupt;
+
 			ctx.peerAddr.family = IP6;
 			ctx.peerAddr.v6     = bgp4mp->a32v6.peerAddr;
 
 			offset = sizeof(bgp4mp->a32v6);
 		}
 	} else {
+		if (len < sizeof(bgp4mp->a16)) goto corrupt;
+
 		ctx.peerAs = beswap16(bgp4mp->a16.peerAs);
 		if (bgp4mp->a16.afi == AFI_IP) {
+			if (len < sizeof(bgp4mp->a16v4)) goto corrupt;
+
 			ctx.peerAddr.family = IP4;
 			ctx.peerAddr.v4     = bgp4mp->a16v4.peerAddr;
 
@@ -971,6 +994,8 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr    *hdr,
 		} else {
 			assert(bgp4mp->a16.afi == AFI_IP6);
 
+			if (len < sizeof(bgp4mp->a16v6)) goto corrupt;
+
 			ctx.peerAddr.family = IP6;
 			ctx.peerAddr.v6     = bgp4mp->a16v6.peerAddr;
 
@@ -984,10 +1009,13 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr    *hdr,
 	if (BGP4MP_ISSTATECHANGE(hdr->subtype)) {
 		// Dump state change
 		BgpFsmState chng[2];
-		memcpy(chng, (Uint8 *) bgp4mp + offset, sizeof(chng));
 
 		Bufio_Putc(&sb, STATE_CHANGE_MARKER);
 		Bufio_Putc(&sb, SEP_CHAR);
+		if (len < offset + sizeof(chng)) goto corrupt;
+
+		memcpy(chng, (Uint8 *) bgp4mp + offset, sizeof(chng));
+
 		Bufio_Putu(&sb, beswap16(chng[0]));
 		Bufio_Putc(&sb, '-');
 		Bufio_Putu(&sb, beswap16(chng[1]));
@@ -1000,17 +1028,14 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr    *hdr,
 		size_t        nbytes = len - offset;
 
 		nbytes = Bgp_CheckMsgHdr(msg, nbytes, /*allowExtendedSize=*/TRUE);
-		if (nbytes != 0) {
+		if (nbytes == 0) {
-			// Header is OK
-			nbytes -= BGP_HDRSIZ;
-			DumpBgp(&sb, msg->type, msg + 1, nbytes, table, &ctx);
-		} else {
 			// Corrupted BGP4MP with invalid BGP data
 			Bufio_Putc(&sb, UNKNOWN_MARKER);
 			Bufio_Putc(&sb, SEP_CHAR);
-			WarnCorrupted(&sb, &ctx);
+			goto corrupt;
-			DumpMrtInfoTrailer(&sb, &ctx);
 		}
+
+		DumpBgp(&sb, msg->type, msg + 1, nbytes - BGP_HDRSIZ, table, &ctx);
 	} else {
 		// Deprecated/Unknown type
 		Bufio_Putc(&sb, UNKNOWN_MARKER);
@@ -1019,6 +1044,11 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr    *hdr,
 	}
 
 	return Bufio_Flush(&sb);
+
+corrupt:
+	WarnCorrupted(&sb, &ctx);
+	DumpMrtInfoTrailer(&sb, &ctx);
+	return Bufio_Flush(&sb);
 }
 
 static Sint64 Isolario_DumpZebra(const Mrthdr   *hdr,

lonetix/bgp/mrt.c

@@ -16,9 +16,13 @@
 #include <assert.h>
 #include <string.h>
 
-static void *MRT_DATAPTR(const Mrtrecord *rec)
+static void *MRT_DATAPTR(const Mrtrecord *rec, size_t *len)
 {
-	return rec->buf + MRT_HDRSIZ + (MRT_ISEXHDRTYPE(MRT_HDR(rec)->type) << 2);
+	const Mrthdr *hdr = MRT_HDR(rec);
+	Uint32        off = MRT_ISEXHDRTYPE(hdr->type) << 2;
+
+	*len = beswap32(hdr->len) - off;
+	return rec->buf + MRT_HDRSIZ + off;
 }
 
 Judgement Bgp_MrtFromBuf(Mrtrecord *rec, const void *buf, size_t nbytes)
@@ -28,9 +32,9 @@ Judgement Bgp_MrtFromBuf(Mrtrecord *rec, const void *buf, size_t nbytes)
 
 	const Mrthdr *hdr = (const Mrthdr *) buf;
 
-	size_t left = beswap32(hdr->len);
+	size_t left = beswap32(hdr->len);            // NOTE: Includes extended timestamp size, if any!
-	if (MRT_ISEXHDRTYPE(hdr->type))
+	if (left < 4 && MRT_ISEXHDRTYPE(hdr->type))  // ...so check it
-		left += 4;  // account for extended timestamp
+		return Bgp_SetErrStat(BGPETRUNCMRT);
 
 	size_t siz = sizeof(*hdr) + left;
 	if (siz > nbytes)
@@ -63,12 +67,12 @@ Judgement Bgp_ReadMrt(Mrtrecord *rec, void *streamp, const StmOps *ops)
 	if ((size_t) n != sizeof(hdr))
 		return Bgp_SetErrStat(BGPEIO);
 
-	size_t left = beswap32(hdr.len);
+	size_t left = beswap32(hdr.len);            // NOTE: Includes extended timestamp size, if any!
-	if (MRT_ISEXHDRTYPE(hdr.type))
+	if (left < 4 && MRT_ISEXHDRTYPE(hdr.type))  // ...so check it
-		left += 4;  // account for extended timestamp
+		return Bgp_SetErrStat(BGPETRUNCMRT);
 
 	// Allocate buffer
-	// NOTE: MRT header length doesn't account for header size itself
+	// NOTE: ...but MRT header length doesn't account for header size itself
 	size_t siz           = sizeof(hdr) + left;
 	const MemOps *memOps = MRT_MEMOPS(rec);
 
@@ -517,11 +521,12 @@ Bgp4mphdr *Bgp_GetBgp4mpHdr(Mrtrecord *rec, size_t *nbytes)
 		return NULL;
 	}
 
-	size_t len    = beswap32(hdr->len);
+	size_t len;
-	size_t offset = 0;
 	Afi    afi;
 
-	Bgp4mphdr *bgp4mp = (Bgp4mphdr *) MRT_DATAPTR(rec);
+	size_t offset = 0;
+
+	Bgp4mphdr *bgp4mp = (Bgp4mphdr *) MRT_DATAPTR(rec, &len);
 	if (BGP4MP_ISASN32BIT(hdr->subtype)) {
 		offset += 2 * 4;
 		offset += 2 + 2;
@@ -572,8 +577,8 @@ Judgement Bgp_UnwrapBgp4mp(Mrtrecord *rec, Bgpmsg *dest, unsigned flags)
 	if (!BGP4MP_ISMESSAGE(hdr->subtype))
 		return Bgp_SetErrStat(BGPEBADMRTTYPE);
 
-	Uint32 len  = beswap32(hdr->len);
+	size_t len;
-	Uint8 *base = (Uint8 *) MRT_DATAPTR(rec);
+	Uint8 *base = (Uint8 *) MRT_DATAPTR(rec, &len);
 
 	// Skip header
 	size_t siz = BGP4MP_ISASN32BIT(hdr->subtype) ? 2*4 : 2*2;  // skip ASN

lonetix/include/df/bgp/mrt.h

@@ -678,7 +678,7 @@ typedef ALIGNED(1, union) {
 
 FORCE_INLINE Bgp4mphdr *BGP4MP_HDR(const Mrthdr *hdr)
 {
-	return (hdr->subtype == MRT_BGP4MP_ET) ?
+	return (hdr->type == MRT_BGP4MP_ET) ?
 	       (Bgp4mphdr *) ((const Mrthdrex *) hdr + 1) :
 	       (Bgp4mphdr *) (hdr + 1);
 }

tools/bgpgrep/bgpgrep_dump.c

@@ -65,6 +65,12 @@ static void FixBgpAttributeTableForRib(Bgpattrtab tab, Boolean isRibv2)
 	}
 }
 
+static void NormalizeExtendedTimestamp(void)
+{
+	S.timestampSecs      += S.timestampMicrosecs / 1000000;
+	S.timestampMicrosecs %= 1000000;
+}
+
 static void OutputBgp4mp(const Mrthdr *hdr, Bgpattrtab tab)
 {
 	S.lenientBgpErrors = TRUE;
@@ -90,8 +96,10 @@ void BgpgrepD_Bgp4mp(void)
 	S.peerAs             = BGP4MP_GETPEERADDR(hdr->subtype, &S.peerAddr, bgp4mp);
 	S.timestampSecs      = beswap32(hdr->timestamp);
 	S.timestampMicrosecs = 0;
-	if (hdr->subtype == MRT_BGP4MP_ET)
+	if (hdr->type == MRT_BGP4MP_ET) {
 		S.timestampMicrosecs = beswap32(((const Mrthdrex *) hdr)->microsecs);
+		NormalizeExtendedTimestamp();
+	}
 
 	// Dump MRT data
 	Bgp_UnwrapBgp4mp(&S.rec, &S.msg, /*flags=*/BGPF_UNOWNED);

tools/bgpgrep/bgpgrep_timestamp.c

@@ -44,12 +44,12 @@ static Uint32 ParseFrac(const char *s, const char *p, char **ep)
 		FATALF("%s: Expecting fractional portion of a second after '.'", s);
 
 	int         ndigs = endp - p;
-	double frac  = pow(10, ndigs);
+	long double frac  = pow(10, ndigs);
 
 	if (ep)
 		*ep = endp;
 
-	return (Uint32) trunc((n / frac) * 100000.0);
+	return (Uint32) truncl(n / frac * 1000000.0L);
 }
 
 static Boolean ParseRfc3339(const char *s, Timestampop *dest)