From: Donald Sharp Date: Thu, 3 May 2018 00:12:31 +0000 (-0400) Subject: pbrd: Fix nearly impossible truncation X-Git-Tag: frr-6.1-dev~479^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=29d5a146344b47ce46217c84daf28b1a0cee1185;p=matthieu%2Ffrr.git pbrd: Fix nearly impossible truncation Since we are writing into the name field which is PBR_MAP_NAMELEN size, we are expecting this to field to be at max 100 bytes. Newer compilers understand that the %s portion may be up to 100 bytes( because of the size of the string. The %u portion is expected to be 10 bytes. So in `theory` there are situations where we might truncate. The reality this is never going to happen( who is going to create a nexthop group name that is over say 30 characters? ). As such we are expecting the calling function to subtract 10 from the size_t l before we pass it in to get around this new gcc fun. Fixes: #2163 Signed-off-by: Donald Sharp --- diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index 1ccf3ebffa..5be96e86d0 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -470,6 +470,18 @@ void pbr_nht_change_group(const char *name) pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg); } +/* + * Since we are writing into the name field which is PBR_MAP_NAMELEN + * size, we are expecting this to field to be at max 100 bytes. + * Newer compilers understand that the %s portion may be up to + * 100 bytes( because of the size of the string. The %u portion + * is expected to be 10 bytes. So in `theory` there are situations + * where we might truncate. The reality this is never going to + * happen( who is going to create a nexthop group name that is + * over say 30 characters? ). As such we are expecting the + * calling function to subtract 10 from the size_t l before + * we pass it in to get around this new gcc fun. + */ char *pbr_nht_nexthop_make_name(char *name, size_t l, uint32_t seqno, char *buffer) { @@ -485,7 +497,7 @@ void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms) struct pbr_nexthop_cache lookup; memset(&find, 0, sizeof(find)); - pbr_nht_nexthop_make_name(pbrms->parent->name, PBR_MAP_NAMELEN, + pbr_nht_nexthop_make_name(pbrms->parent->name, PBR_MAP_NAMELEN - 10, pbrms->seqno, find.name); if (!pbrms->internal_nhg_name) pbrms->internal_nhg_name = XSTRDUP(MTYPE_TMP, find.name); diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c index 03f2104835..ba5c49ad5c 100644 --- a/pbrd/pbr_vty.c +++ b/pbrd/pbr_vty.c @@ -269,7 +269,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, if (pbrms->nhg) nh = nexthop_exists(pbrms->nhg, &nhop); else { - char buf[100]; + char buf[PBR_MAP_NAMELEN]; if (no) { vty_out(vty, "No nexthops to delete"); @@ -280,7 +280,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, pbrms->internal_nhg_name = XSTRDUP(MTYPE_TMP, pbr_nht_nexthop_make_name(pbrms->parent->name, - PBR_MAP_NAMELEN, + PBR_MAP_NAMELEN - 10, pbrms->seqno, buf)); nh = NULL;