Re: [PATCH v4] scsi: target: iscsi: validate CHAP_R length before base64 decode

On Thu, 21 May 2026 17:11:21 +0200, Alexandru Hossu wrote:

> chap_server_compute_hash() allocates client_digest as
> kzalloc(chap->digest_size) and then, for BASE64-encoded responses,
> passes chap_r directly to chap_base64_decode() without checking whether
> the input length could produce more than digest_size bytes of output.
> 
> chap_base64_decode() writes to the destination unconditionally as long
> as there is input to consume. With MAX_RESPONSE_LENGTH set to 128 and
> the "0b" prefix stripped by extract_param(), up to 127 base64 characters
> can reach the decoder. 127 characters decode to 95 bytes. For SHA-256
> (digest_size=32) this overflows client_digest by 63 bytes; for MD5
> (digest_size=16) the overflow is 79 bytes.
> 
> The length check at line 344 fires after the write has already happened.
> 
> The HEX branch in the same switch statement already validates the length
> up front. Apply the same approach to the BASE64 branch: strip trailing
> base64 padding characters, then reject any input whose data length
> exceeds DIV_ROUND_UP(digest_size * 4, 3) before calling the decoder.
> 
> Stripping trailing '=' before the comparison handles both padded and
> unpadded encodings. chap_base64_decode() already returns early on '=',
> so the full original string is still passed to the decoder unchanged.
> 
> The mutual CHAP path decodes CHAP_C into initiatorchg_binhex, which is
> kzalloc(CHAP_CHALLENGE_STR_LEN). extract_param() caps initiatorchg at
> CHAP_CHALLENGE_STR_LEN characters, so at most CHAP_CHALLENGE_STR_LEN-1
> base64 characters reach the decoder. The maximum decoded size,
> DIV_ROUND_UP((CHAP_CHALLENGE_STR_LEN-1) * 3, 4), is less than
> CHAP_CHALLENGE_STR_LEN, so no overflow is possible there. A comment is
> added at the call site to document this.
> 
> Fixes: 1e5733883421 ("scsi: target: iscsi: Support base64 in CHAP")
> Cc: [email protected]
> Signed-off-by: Alexandru Hossu <[email protected]>

Reviewed-by: David Disseldorp <[email protected]>