On Tue, Sep 08, 2015 at 09:45:06PM +0200, Tobias Stoeckmann wrote:
> The function pr_pack does not properly check boundaries before
> accessing packet data. This could happen on short network reads or
> when we receive packets that are addressed for another running ping6
> instance (see pr_pack comment for more information).
>
> Index: ping6.c
> ===================================================================
> RCS file: /cvs/src/sbin/ping6/ping6.c,v
> retrieving revision 1.112
> diff -u -p -r1.112 ping6.c
> --- ping6.c 1 Sep 2015 19:53:23 -0000 1.112
> +++ ping6.c 8 Sep 2015 19:29:29 -0000
> @@ -1222,6 +1222,11 @@ pr_pack(u_char *buf, int cc, struct msgh
> SIPHASH_CTX ctx;
> u_int8_t mac[SIPHASH_DIGEST_LENGTH];
>
> + if (cc - sizeof(*cp) < sizeof(payload)) {
> + (void)printf("signature missing!\n");
> + return;
> + }
> +
I think we should still output information about the packet, also in
the case that the signature doesn't match. ping is used to debug
problems, it should output as much information as possible when weird
stuff happens.
That will require some reshuffling, so I'm fine with this going in
first.
OK florian@
> memcpy(&payload, icp + 1, sizeof(payload));
> tv64 = &payload.tv64;
>
> @@ -1306,7 +1311,8 @@ pr_pack(u_char *buf, int cc, struct msgh
> }
> }
> }
> - } else if (icp->icmp6_type == ICMP6_NI_REPLY && mynireply(ni)) {
> + } else if (icp->icmp6_type == ICMP6_NI_REPLY &&
> + cc >= sizeof(*ni) && mynireply(ni)) {
> seq = ntohs(*(u_int16_t *)ni->icmp6_ni_nonce);
> ++nreceived;
> if (TST(seq % mx_dup_ck)) {
> @@ -1351,7 +1357,7 @@ pr_pack(u_char *buf, int cc, struct msgh
> case NI_QTYPE_FQDN:
> default: /* XXX: for backward compatibility */
> cp = (u_char *)ni + ICMP6_NIRLEN;
> - if (buf[off + ICMP6_NIRLEN] ==
> + if (off + ICMP6_NIRLEN < cc && buf[off + ICMP6_NIRLEN]
> ==
> cc - off - ICMP6_NIRLEN - 1)
> oldfqdn = 1;
> else
> @@ -1438,7 +1444,8 @@ pr_pack(u_char *buf, int cc, struct msgh
> }
> }
>
> - if (buf[off + ICMP6_NIRLEN] !=
> + if (off + ICMP6_NIRLEN < cc &&
> + buf[off + ICMP6_NIRLEN] !=
> cc - off - ICMP6_NIRLEN - 1 && oldfqdn) {
> if (comma)
> printf(",");
>
--
I'm not entirely sure you are real.
