Unused var assignment (scan-build warning) #414

1. 
   rustyrussell commented at 2:04 am on September 8, 2016: contributor

    0124 static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp256k1_gej *r, const secp256k1_scalar *gn) {
    1125     secp256k1_ge add;
    2126     secp256k1_ge_storage adds;
    3127     secp256k1_scalar gnb;
    4128     int bits;
    5129     int i, j;
    6130     memset(&adds, 0, sizeof(adds));
    7131     *r = ctx->initial;
    8132     /* Blind scalar/point multiplication by computing (n-b)G + bG instead of nG. */
    9133     secp256k1_scalar_add(&gnb, gn, &ctx->blind);
   10134     add.infinity = 0;
   11135     for (j = 0; j < 64; j++) {
   12136         bits = secp256k1_scalar_get_bits(&gnb, j * 4, 4);
   13137         for (i = 0; i < 16; i++) {
   14138             /** This uses a conditional move to avoid any secret data in array indexes.
   15139              *   _Any_ use of secret indexes has been demonstrated to result in timing
   16140              *   sidechannels, even when the cache-line access patterns are uniform.
   17141              *  See also:
   18142              *   "A word of warning", CHES 2013 Rump Session, by Daniel J. Bernstein and Peter Schwabe
   19143              *    (https://cryptojedi.org/peter/data/chesrump-20130822.pdf) and
   20144              *   "Cache Attacks and Countermeasures: the Case of AES", RSA 2006,
   21145              *    by Dag Arne Osvik, Adi Shamir, and Eran Tromer
   22146              *    (http://www.tau.ac.il/~tromer/papers/cache.pdf)
   23147              */
   24148             secp256k1_ge_storage_cmov(&adds, &(*ctx->prec)[j][i], i == bits);
   25149         }
   26150         secp256k1_ge_from_storage(&add, &adds);
   27151         secp256k1_gej_add_ge(r, r, &add);
   28152     }
   29153     bits = 0;
   30
   31Value stored to 'bits' is never read
   32154     secp256k1_ge_clear(&add);
   33155     secp256k1_scalar_clear(&gnb);
   34156 }
   

2. 
   llamasoft commented at 5:44 pm on September 16, 2016: contributor

   Looks like a false positive.
   During the loop, bits contains private data (4-bit slices from gn). The value is reset at the end of the function to prevent any private data from being read after the function ends.
3. 
   rustyrussell commented at 5:43 am on September 17, 2016: contributor

   llamasoft notifications@github.com writes:

   Looks like a false positive.
   During the loop, bits contains private data (4-bit slices from gn). The value is reset at the end of the function to prevent any private data from being read after the function ends.

   That doesn’t work, the compiler will almost certainly optimize out the assignment. And even if it’s best-effort, it would be best to comment it…

   In fact, from a cursory test with gcc 5.4.0 in x86-64 Ubuntu, you can delete these three lines at the end of secp256k1_ecmult_gen():

   0bits = 0;
   1secp256k1_ge_clear(&add);
   2secp256k1_scalar_clear(&gnb);
   

   And the object produced doesn’t change.

   Cheers, Rusty.

4. llamasoft cross-referenced this on Sep 19, 2016 from issue Compiler optimizing out clearing of sensitive buffers by llamasoft
5. 
   gmaxwell commented at 12:12 pm on May 29, 2019: contributor

   Replaced by more descriptive #418
6. gmaxwell closed this on May 29, 2019

   ---

Contributors
rustyrussell llamasoft gmaxwell