NetBSD Problem Report #57449

From www@netbsd.org  Sat Jun  3 18:17:06 2023
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 892AB1A923F
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  3 Jun 2023 18:17:06 +0000 (UTC)
Message-Id: <20230603181705.0C1181A9241@mollari.NetBSD.org>
Date: Sat,  3 Jun 2023 18:17:05 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: Approximately a bajillion buffer overruns in netpgpverify
X-Send-Pr-Version: www-1.0

>Number:         57449
>Category:       security
>Synopsis:       Approximately a bajillion buffer overruns in netpgpverify
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    security-officer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jun 03 18:20:00 +0000 2023
>Last-Modified:  Sat Jun 03 18:30:02 +0000 2023
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The NetPGP Foundation
>Environment:
>Description:
In read_sig_subpackets:

        for (i = 0 ; (unsigned)(p - start) < sigpkt->subslen ; i++) {
                memset(&subpkt, 0x0, sizeof(subpkt));
                subpkt.s.size = get_pkt_len(1, p, 0, is_subpkt);
                lenlen = get_pkt_len_len(1, p, is_subpkt);
                if (lenlen > pktlen) {
                        printf("weird lenlen %u\n", lenlen);
                        return 0;
                }
                p += lenlen;

The condition (p - start) < sigpkt->subslen guarantees that get_pkt_len_len(..., p, ...), because it only reads p[0].

But it is not enough to guarantee that get_pkt_len(..., p, ...) will work, because that may read p[0], p[1], p[2], or p[3] depending on the value of p[-1].
>How-To-Repeat:
1. Make a signature with gpg or gpg2.
2. Find the 0x89 0x01 sequence.
3. Change it to 0x8a 0x7f.

Now netpgpverify will read this as a signature packet with a length of somewhere between 2^30 and 2^31 bytes and try to read memory out of oblivion.
>Fix:
Yes, please!

1. Find each get_pkt_len_len.
2. Guarantee that the length it returns is within the buffer size _before_ calling anything else like get_pkt_len.
3. Find each get_pkt_len.
4. Guarantee that the length it returns is within the buffer size before doing anything else.

>Audit-Trail:
From: Taylor R Campbell <riastradh@NetBSD.org>
To: gnats-bugs@netbsd.org
Cc: security-officer@netbsd.org, gnats-admin@netbsd.org, security-alert@netbsd.org
Subject: Re: security/57449: Approximately a bajillion buffer overruns in netpgpverify
Date: Sat, 3 Jun 2023 18:28:44 +0000

 I forgot to add: in cases like

                 switch(subpkt.tag) {
                 case SUBPKT_SIG_BIRTH:
                         sigpkt->sig.birth =3D (int64_t)get_32(p);
                         break;
                 case SUBPKT_SIG_EXPIRY:
                         sigpkt->sig.expiry =3D (int64_t)get_32(p);
                         break;
                 case SUBPKT_KEY_EXPIRY:
                         sigpkt->sig.keyexpiry =3D (int64_t)get_32(p);
                         break;
                 case SUBPKT_ISSUER:
                         memcpy(sigpkt->sig.signer, p, sizeof(sigpkt->sig.si=
 gner));
                         break;
                 case SUBPKT_SIGNER_ID:
                         memcpy(sigpkt->sig.signer, p, sizeof(sigpkt->sig.si=
 gner));
                         break;

 every one of the cases is a potential buffer overrun, because nothing
 verifies that the buffer that p points to is long enough for
 get_32(p), memcpy(..., p, sizeof(sigpkt->sig.signer)), &c.  Every one
 of these needs its own bounds check too.  Hence `approximately a
 bajillion'.

 (Unrelated bug: Apparently the gnats web interface doesn't support
 submitting confidential PRs.)

Home	PR Database Search