From: Alexandru Hossu <hossu.alexandru@gmail.com>
To: d.bogdanov@yadro.com
Cc: mlombard@arkamax.eu, martin.petersen@oracle.com,
bvanassche@acm.org, ddiss@suse.de, target-devel@vger.kernel.org,
linux-scsi@vger.kernel.org, stable@vger.kernel.org,
hossu.alexandru@gmail.com
Subject: Re: [PATCH v2] scsi: target: iscsi: validate CHAP_R length before base64 decode
Date: Wed, 20 May 2026 17:43:38 -0700 (PDT) [thread overview]
Message-ID: <6a0e553a.010ccaa2.2ab173.fc09@mx.google.com> (raw)
In-Reply-To: <20260520180204.GA15940@yadro.com>
On Wed, May 20, 2026, Dmitry Bogdanov <d.bogdanov@yadro.com> wrote:
> Yes, the length of Base64 decoded string is not deterministic.
> Moreover, length of Base64 encoded string must be divisible by 4. Which
> is biger that 4/3 of decoded.
>
> | MD5_SIGNATURE_SIZE | 16 | 21,33333 | 22 | 24 |
> | SHA256_SIGNATURE_SIZE | 32 | 42,66667 | 43 | 44 |
>
> So, that formula is not correct and will break all iscsi authentication.
v3 (sent about an hour before your email) already handles this. Trailing
'=' are stripped before the comparison, so the check is applied only to
the data characters:
while (r_len > 0 && chap_r[r_len - 1] == '=')
r_len--;
if (r_len > DIV_ROUND_UP(chap->digest_size * 4, 3)) {
Using your table as input:
MD5 padded: "data==" -> r_len = 24-2 = 22, 22 <= 22 ✓
SHA-256 padded: "data=" -> r_len = 44-1 = 43, 43 <= 43 ✓
> Alexandru, may be better just to change size of client_diggest variable
> to match it with chap_r like for initiatorchg and initiatorchg_binhex?
That also prevents the overflow. The length check is preferred for
consistency with the HEX branch, which validates input length before
calling the decoder rather than relying on a larger destination buffer.
Alexandru
next prev parent reply other threads:[~2026-05-21 0:43 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-18 12:18 [PATCH] scsi: target: iscsi: validate CHAP_R length before base64 decode Alexandru Hossu
2026-05-18 14:40 ` David Disseldorp
2026-05-18 23:50 ` [PATCH v2] " Alexandru Hossu
2026-05-20 15:56 ` Maurizio Lombardi
2026-05-20 16:53 ` Alexandru Hossu
2026-05-20 18:02 ` Dmitry Bogdanov
2026-05-21 0:43 ` Alexandru Hossu [this message]
2026-05-18 23:51 ` [PATCH] " Alexandru Hossu
2026-05-20 16:52 ` [PATCH v3] " Alexandru Hossu
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=6a0e553a.010ccaa2.2ab173.fc09@mx.google.com \
--to=hossu.alexandru@gmail.com \
--cc=bvanassche@acm.org \
--cc=d.bogdanov@yadro.com \
--cc=ddiss@suse.de \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mlombard@arkamax.eu \
--cc=stable@vger.kernel.org \
--cc=target-devel@vger.kernel.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