Unused var assignment (scan-build warning) #414

124 static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp256k1_gej *r, const secp256k1_scalar *gn) {
125     secp256k1_ge add;
126     secp256k1_ge_storage adds;
127     secp256k1_scalar gnb;
128     int bits;
129     int i, j;
130     memset(&adds, 0, sizeof(adds));
131     *r = ctx->initial;
132     /* Blind scalar/point multiplication by computing (n-b)G + bG instead of nG. */
133     secp256k1_scalar_add(&gnb, gn, &ctx->blind);
134     add.infinity = 0;
135     for (j = 0; j < 64; j++) {
136         bits = secp256k1_scalar_get_bits(&gnb, j * 4, 4);
137         for (i = 0; i < 16; i++) {
138             /** This uses a conditional move to avoid any secret data in array indexes.
139              *   _Any_ use of secret indexes has been demonstrated to result in timing
140              *   sidechannels, even when the cache-line access patterns are uniform.
141              *  See also:
142              *   "A word of warning", CHES 2013 Rump Session, by Daniel J. Bernstein and Peter Schwabe
143              *    (https://cryptojedi.org/peter/data/chesrump-20130822.pdf) and
144              *   "Cache Attacks and Countermeasures: the Case of AES", RSA 2006,
145              *    by Dag Arne Osvik, Adi Shamir, and Eran Tromer
146              *    (http://www.tau.ac.il/~tromer/papers/cache.pdf)
147              */
148             secp256k1_ge_storage_cmov(&adds, &(*ctx->prec)[j][i], i == bits);
149         }
150         secp256k1_ge_from_storage(&add, &adds);
151         secp256k1_gej_add_ge(r, r, &add);
152     }
153     bits = 0;

Value stored to 'bits' is never read
154     secp256k1_ge_clear(&add);
155     secp256k1_scalar_clear(&gnb);
156 }

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.

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():

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

And the object produced doesn't change.

Cheers, Rusty.

Replaced by more descriptive #418