From: Patrick Steinhardt <ps@pks.im>
To: Beat Bolli <dev+git@drbeat.li>
Cc: git@vger.kernel.org, Oswald Buddenhagen <ossi@kde.org>
Subject: Re: [PATCH 2/3] imap-send: don't expect an ASN1_STRING to be NUL-terminated
Date: Tue, 8 Sep 2026 10:26:43 +0200 [thread overview]
Message-ID: <ap_Gwx4g7t0vjsj2@pks.im> (raw)
In-Reply-To: <20260907211210.2621693-3-dev+git@drbeat.li>
On Mon, Sep 07, 2026 at 11:12:09PM +0200, Beat Bolli wrote:
> diff --git a/imap-send.c b/imap-send.c
> index 977d78005c..9a807cdde8 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -226,20 +226,25 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED,
>
> static int host_matches(const char *host, const ASN1_STRING *asn1_str)
> {
> - const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
> + int ret = 0;
> + size_t len = ASN1_STRING_get_length(asn1_str);
> + char *pattern = xmemdupz(ASN1_STRING_get0_data(asn1_str), len);
>
> /* embedded NUL characters may open a security hole */
> - if (memchr(pattern, '\0', ASN1_STRING_get_length(asn1_str)))
> - return 0;
> + if (memchr(pattern, '\0', len))
> + goto out;
>
> if (pattern[0] == '*' && pattern[1] == '.') {
> pattern += 2;
> if (!(host = strchr(host, '.')))
> - return 0;
> + goto out;
> host++;
> }
>
> - return *host && *pattern && !strcasecmp(host, pattern);
> + ret = *host && *pattern && !strcasecmp(host, pattern);
> +out:
> + free(pattern);
> + return ret;
> }
I don't quite see a reason why we even have to memdup the string. We
already use memchr, which is bounded by the length of the string. We do
have two other sites though:
- We use strchr, but that can be adapted to use memchr.
- Likewise, we use strcasecmp, but that can be adapted to use
strncasecmp.
So with that, all calls that inspect the string would be bounded by the
length of the encoded string, and that means we don't have to copy the
string first, do we?
Patrick
next prev parent reply other threads:[~2026-09-08 8:26 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-07 21:12 [PATCH 0/3] imap-send: future proofing and two correctness fixes Beat Bolli
2026-09-07 21:12 ` [PATCH 1/3] imap-send: prepare for OpenSSL 4.1 Beat Bolli
2026-09-08 4:17 ` Junio C Hamano
2026-09-08 8:26 ` Patrick Steinhardt
2026-09-07 21:12 ` [PATCH 2/3] imap-send: don't expect an ASN1_STRING to be NUL-terminated Beat Bolli
2026-09-08 1:17 ` Junio C Hamano
2026-09-08 8:26 ` Patrick Steinhardt [this message]
2026-09-07 21:12 ` [PATCH 3/3] imap-send: only check the CN if no SAN DNS names are present Beat Bolli
2026-09-08 1:28 ` brian m. carlson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ap_Gwx4g7t0vjsj2@pks.im \
--to=ps@pks.im \
--cc=dev+git@drbeat.li \
--cc=git@vger.kernel.org \
--cc=ossi@kde.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox