Skip to content

Commit 85db739

Browse files
ahossumartinkpetersen
authored andcommitted
scsi: target: iscsi: Validate CHAP_R length before base64 decode
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: 1e57338 ("scsi: target: iscsi: Support base64 in CHAP") Cc: stable@vger.kernel.org Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com> Reviewed-by: David Disseldorp <ddiss@suse.de> Link: https://patch.msgid.link/20260521151121.808477-1-hossu.alexandru@gmail.com Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent bf33e01 commit 85db739

1 file changed

Lines changed: 18 additions & 1 deletion

File tree

drivers/target/iscsi/iscsi_target_auth.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,13 +340,22 @@ static int chap_server_compute_hash(
340340
goto out;
341341
}
342342
break;
343-
case BASE64:
343+
case BASE64: {
344+
size_t r_len = strlen(chap_r);
345+
346+
while (r_len > 0 && chap_r[r_len - 1] == '=')
347+
r_len--;
348+
if (r_len > DIV_ROUND_UP(chap->digest_size * 4, 3)) {
349+
pr_err("Malformed CHAP_R: base64 payload too long\n");
350+
goto out;
351+
}
344352
if (chap_base64_decode(client_digest, chap_r, strlen(chap_r)) !=
345353
chap->digest_size) {
346354
pr_err("Malformed CHAP_R: invalid BASE64\n");
347355
goto out;
348356
}
349357
break;
358+
}
350359
default:
351360
pr_err("Could not find CHAP_R\n");
352361
goto out;
@@ -473,6 +482,14 @@ static int chap_server_compute_hash(
473482
}
474483
break;
475484
case BASE64:
485+
/*
486+
* No overflow check needed: initiatorchg_binhex is
487+
* CHAP_CHALLENGE_STR_LEN bytes and extract_param() caps
488+
* initiatorchg at CHAP_CHALLENGE_STR_LEN characters, so
489+
* the decoded output is at most DIV_ROUND_UP(
490+
* (CHAP_CHALLENGE_STR_LEN - 1) * 3, 4) bytes, which is
491+
* less than CHAP_CHALLENGE_STR_LEN.
492+
*/
476493
initiatorchg_len = chap_base64_decode(initiatorchg_binhex,
477494
initiatorchg,
478495
strlen(initiatorchg));

0 commit comments

Comments
 (0)