intel / intel-ipsec-mb

Intel(R) Multi-Buffer Crypto for IPSec
BSD 3-Clause "New" or "Revised" License
288 stars 87 forks source link

potential uninitialized value error found by static analysis #76

Closed ColinIanKing closed 3 years ago

ColinIanKing commented 3 years ago

Static analysis has reported a potential issue, I'm not sure if it is a false positive.

File: lib/include/mb_mgr_code.h

431submit_snow_v_aead_job(IMB_MGR *state, IMB_JOB *job)
432{
433        struct gcm_key_data gdata_key;
434        imb_uint128_t *auth = (imb_uint128_t *) job->auth_tag_output;
435        imb_uint128_t temp;
   1. var_decl: Declaring variable hkey_endpad without initializer.
436        imb_uint128_t hkey_endpad[2];
437
438        temp.low = BSWAP64((job->u.SNOW_V_AEAD.aad_len_in_bytes << 3));
439        temp.high = BSWAP64((job->msg_len_to_cipher_in_bytes << 3));
440
441        /* if hkey_endpad[1].high == 0:
442         *      SUBMIT_JOB_SNOW_V_AEAD does enc/decrypt operation
443         *      and fills hkey_endpad with first 2 keystreams
444         * else
445         *      SUBMIT_JOB_SNOW_V_AEAD fills hkey_endpad with first
446         *      2 keystreams (no operations on src vector are done)
447         */
   2. Condition job->cipher_direction == IMB_DIR_ENCRYPT, taking true branch.
448        if(job->cipher_direction == IMB_DIR_ENCRYPT)
   3. Falling through to end of if statement.
449                hkey_endpad[1].high = 0;
450        else
451                hkey_endpad[1].high = 1;
452
453        job->u.SNOW_V_AEAD.reserved = hkey_endpad;
454        job = SUBMIT_JOB_SNOW_V_AEAD(job);
455
456        memset(auth, 0, sizeof(imb_uint128_t));
457
458        /* GHASH key H */
459        IMB_GHASH_PRE(state, (void *)hkey_endpad,  &gdata_key);
460
461        /* push AAD into GHASH */
462        IMB_GHASH(state, &gdata_key, job->u.SNOW_V_AEAD.aad,
463                job->u.SNOW_V_AEAD.aad_len_in_bytes,
464                (void *)auth, sizeof(imb_uint128_t));
465
   4. Condition job->cipher_direction == IMB_DIR_ENCRYPT, taking true branch.
466        if (job->cipher_direction == IMB_DIR_ENCRYPT)
467                IMB_GHASH(state, &gdata_key, job->dst,
468                        job->msg_len_to_cipher_in_bytes,
   5. Falling through to end of if statement.
469                        (void *)auth, sizeof(imb_uint128_t));
470        else
471                IMB_GHASH(state, &gdata_key, job->src,
472                        job->msg_len_to_cipher_in_bytes,
473                        (void *)auth, sizeof(imb_uint128_t));
474
475        IMB_GHASH(state, &gdata_key, (void *)&temp, sizeof(temp),
476                  (void *)auth, sizeof(imb_uint128_t));
477
478        /* The resulting AuthTag */
   CID 113803 (#1 of 1): Uninitialized scalar variable (UNINIT)
   6. uninit_use: Using uninitialized value hkey_endpad[1].low.
479        auth->low = auth->low ^ hkey_endpad[1].low;
480        auth->high = auth->high ^ hkey_endpad[1].high;
481
482        if (job->cipher_direction == IMB_DIR_DECRYPT) {
483                hkey_endpad[1].high = 0;
484                job = SUBMIT_JOB_SNOW_V_AEAD(job);
485        }
486        return job;
487}
tkanteck commented 3 years ago

Thanks Colin. This one is a false positive. This structure is set up in assembly code (call at line 454) that static analysis tool doesn't see here.