lkml.org 
[lkml]   [2018]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.18 011/120] scsi: target: iscsi: Use hex2bin instead of a re-implementation
    Date
    3.18-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Vincent Pelletier <plr.vincent@gmail.com>

    commit 1816494330a83f2a064499d8ed2797045641f92c upstream.

    This change has the following effects, in order of descreasing importance:

    1) Prevent a stack buffer overflow

    2) Do not append an unnecessary NULL to an anyway binary buffer, which
    is writing one byte past client_digest when caller is:
    chap_string_to_hex(client_digest, chap_r, strlen(chap_r));

    The latter was found by KASAN (see below) when input value hes expected size
    (32 hex chars), and further analysis revealed a stack buffer overflow can
    happen when network-received value is longer, allowing an unauthenticated
    remote attacker to smash up to 17 bytes after destination buffer (16 bytes
    attacker-controlled and one null). As switching to hex2bin requires
    specifying destination buffer length, and does not internally append any null,
    it solves both issues.

    This addresses CVE-2018-14633.

    Beyond this:

    - Validate received value length and check hex2bin accepted the input, to log
    this rejection reason instead of just failing authentication.

    - Only log received CHAP_R and CHAP_C values once they passed sanity checks.

    ==================================================================
    BUG: KASAN: stack-out-of-bounds in chap_string_to_hex+0x32/0x60 [iscsi_target_mod]
    Write of size 1 at addr ffff8801090ef7c8 by task kworker/0:0/1021

    CPU: 0 PID: 1021 Comm: kworker/0:0 Tainted: G O 4.17.8kasan.sess.connops+ #2
    Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 05/19/2014
    Workqueue: events iscsi_target_do_login_rx [iscsi_target_mod]
    Call Trace:
    dump_stack+0x71/0xac
    print_address_description+0x65/0x22e
    ? chap_string_to_hex+0x32/0x60 [iscsi_target_mod]
    kasan_report.cold.6+0x241/0x2fd
    chap_string_to_hex+0x32/0x60 [iscsi_target_mod]
    chap_server_compute_md5.isra.2+0x2cb/0x860 [iscsi_target_mod]
    ? chap_binaryhex_to_asciihex.constprop.5+0x50/0x50 [iscsi_target_mod]
    ? ftrace_caller_op_ptr+0xe/0xe
    ? __orc_find+0x6f/0xc0
    ? unwind_next_frame+0x231/0x850
    ? kthread+0x1a0/0x1c0
    ? ret_from_fork+0x35/0x40
    ? ret_from_fork+0x35/0x40
    ? iscsi_target_do_login_rx+0x3bc/0x4c0 [iscsi_target_mod]
    ? deref_stack_reg+0xd0/0xd0
    ? iscsi_target_do_login_rx+0x3bc/0x4c0 [iscsi_target_mod]
    ? is_module_text_address+0xa/0x11
    ? kernel_text_address+0x4c/0x110
    ? __save_stack_trace+0x82/0x100
    ? ret_from_fork+0x35/0x40
    ? save_stack+0x8c/0xb0
    ? 0xffffffffc1660000
    ? iscsi_target_do_login+0x155/0x8d0 [iscsi_target_mod]
    ? iscsi_target_do_login_rx+0x3bc/0x4c0 [iscsi_target_mod]
    ? process_one_work+0x35c/0x640
    ? worker_thread+0x66/0x5d0
    ? kthread+0x1a0/0x1c0
    ? ret_from_fork+0x35/0x40
    ? iscsi_update_param_value+0x80/0x80 [iscsi_target_mod]
    ? iscsit_release_cmd+0x170/0x170 [iscsi_target_mod]
    chap_main_loop+0x172/0x570 [iscsi_target_mod]
    ? chap_server_compute_md5.isra.2+0x860/0x860 [iscsi_target_mod]
    ? rx_data+0xd6/0x120 [iscsi_target_mod]
    ? iscsit_print_session_params+0xd0/0xd0 [iscsi_target_mod]
    ? cyc2ns_read_begin.part.2+0x90/0x90
    ? _raw_spin_lock_irqsave+0x25/0x50
    ? memcmp+0x45/0x70
    iscsi_target_do_login+0x875/0x8d0 [iscsi_target_mod]
    ? iscsi_target_check_first_request.isra.5+0x1a0/0x1a0 [iscsi_target_mod]
    ? del_timer+0xe0/0xe0
    ? memset+0x1f/0x40
    ? flush_sigqueue+0x29/0xd0
    iscsi_target_do_login_rx+0x3bc/0x4c0 [iscsi_target_mod]
    ? iscsi_target_nego_release+0x80/0x80 [iscsi_target_mod]
    ? iscsi_target_restore_sock_callbacks+0x130/0x130 [iscsi_target_mod]
    process_one_work+0x35c/0x640
    worker_thread+0x66/0x5d0
    ? flush_rcu_work+0x40/0x40
    kthread+0x1a0/0x1c0
    ? kthread_bind+0x30/0x30
    ret_from_fork+0x35/0x40

    The buggy address belongs to the page:
    page:ffffea0004243bc0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
    flags: 0x17fffc000000000()
    raw: 017fffc000000000 0000000000000000 0000000000000000 00000000ffffffff
    raw: ffffea0004243c20 ffffea0004243ba0 0000000000000000 0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
    ffff8801090ef680: f2 f2 f2 f2 f2 f2 f2 01 f2 f2 f2 f2 f2 f2 f2 00
    ffff8801090ef700: f2 f2 f2 f2 f2 f2 f2 00 02 f2 f2 f2 f2 f2 f2 00
    >ffff8801090ef780: 00 f2 f2 f2 f2 f2 f2 00 00 f2 f2 f2 f2 f2 f2 00
    ^
    ffff8801090ef800: 00 f2 f2 f2 f2 f2 f2 00 00 00 00 02 f2 f2 f2 f2
    ffff8801090ef880: f2 f2 f2 00 00 00 00 00 00 00 00 f2 f2 f2 f2 00
    ==================================================================

    Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
    Reviewed-by: Mike Christie <mchristi@redhat.com>
    Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/target/iscsi/iscsi_target_auth.c | 30 ++++++++++++++----------------
    1 file changed, 14 insertions(+), 16 deletions(-)

    --- a/drivers/target/iscsi/iscsi_target_auth.c
    +++ b/drivers/target/iscsi/iscsi_target_auth.c
    @@ -26,18 +26,6 @@
    #include "iscsi_target_nego.h"
    #include "iscsi_target_auth.h"

    -static int chap_string_to_hex(unsigned char *dst, unsigned char *src, int len)
    -{
    - int j = DIV_ROUND_UP(len, 2), rc;
    -
    - rc = hex2bin(dst, src, j);
    - if (rc < 0)
    - pr_debug("CHAP string contains non hex digit symbols\n");
    -
    - dst[j] = '\0';
    - return j;
    -}
    -
    static void chap_binaryhex_to_asciihex(char *dst, char *src, int src_len)
    {
    int i;
    @@ -241,9 +229,16 @@ static int chap_server_compute_md5(
    pr_err("Could not find CHAP_R.\n");
    goto out;
    }
    + if (strlen(chap_r) != MD5_SIGNATURE_SIZE * 2) {
    + pr_err("Malformed CHAP_R\n");
    + goto out;
    + }
    + if (hex2bin(client_digest, chap_r, MD5_SIGNATURE_SIZE) < 0) {
    + pr_err("Malformed CHAP_R\n");
    + goto out;
    + }

    pr_debug("[server] Got CHAP_R=%s\n", chap_r);
    - chap_string_to_hex(client_digest, chap_r, strlen(chap_r));

    tfm = crypto_alloc_hash("md5", 0, CRYPTO_ALG_ASYNC);
    if (IS_ERR(tfm)) {
    @@ -348,9 +343,7 @@ static int chap_server_compute_md5(
    pr_err("Could not find CHAP_C.\n");
    goto out;
    }
    - pr_debug("[server] Got CHAP_C=%s\n", challenge);
    - challenge_len = chap_string_to_hex(challenge_binhex, challenge,
    - strlen(challenge));
    + challenge_len = DIV_ROUND_UP(strlen(challenge), 2);
    if (!challenge_len) {
    pr_err("Unable to convert incoming challenge\n");
    goto out;
    @@ -359,6 +352,11 @@ static int chap_server_compute_md5(
    pr_err("CHAP_C exceeds maximum binary size of 1024 bytes\n");
    goto out;
    }
    + if (hex2bin(challenge_binhex, challenge, challenge_len) < 0) {
    + pr_err("Malformed CHAP_C\n");
    + goto out;
    + }
    + pr_debug("[server] Got CHAP_C=%s\n", challenge);
    /*
    * During mutual authentication, the CHAP_C generated by the
    * initiator must not match the original CHAP_C generated by

    \
     
     \ /
      Last update: 2018-10-11 18:05    [W:2.543 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site