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: 1e5733883421 ("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>

diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c
index c46c69a28e97eb0c6171d665b33eb374eeb4379c..a3ad2d244dbee1a3f50b9a86a8140f2ba7114741 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -340,13 +340,22 @@ static int chap_server_compute_hash(
                        goto out;
                }
                break;
-       case BASE64:
+       case BASE64: {
+               size_t r_len = strlen(chap_r);
+
+               while (r_len > 0 && chap_r[r_len - 1] == '=')
+                       r_len--;
+               if (r_len > DIV_ROUND_UP(chap->digest_size * 4, 3)) {
+                       pr_err("Malformed CHAP_R: base64 payload too long\n");
+                       goto out;
+               }
                if (chap_base64_decode(client_digest, chap_r, strlen(chap_r)) !=
                    chap->digest_size) {
                        pr_err("Malformed CHAP_R: invalid BASE64\n");
                        goto out;
                }
                break;
+       }
        default:
                pr_err("Could not find CHAP_R\n");
                goto out;
@@ -473,6 +482,14 @@ static int chap_server_compute_hash(
                }
                break;
        case BASE64:
+               /*
+                * No overflow check needed: initiatorchg_binhex is
+                * CHAP_CHALLENGE_STR_LEN bytes and extract_param() caps
+                * initiatorchg at CHAP_CHALLENGE_STR_LEN characters, so
+                * the decoded output is at most DIV_ROUND_UP(
+                * (CHAP_CHALLENGE_STR_LEN - 1) * 3, 4) bytes, which is
+                * less than CHAP_CHALLENGE_STR_LEN.
+                */
                initiatorchg_len = chap_base64_decode(initiatorchg_binhex,
                                                      initiatorchg,
                                                      strlen(initiatorchg));