Unused var assignment (scan-build warning) #414

issue rustyrussell openend this issue on September 8, 2016
  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


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 05:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me