fpe
fpe copied to clipboard
capitalone
Unused `R` in decryption
Looking here:
https://github.com/capitalone/fpe/blob/e6bfa0dae121111c2e93483777f0f2cde562f341/ff1/ff1.go#L484
The comment suggests R should be used but it is entirely unused. (As reported by staticcheck here). Is this dead/delete-able or does it indicate a decryption error?
It's strange that staticcheck considers it unused, because it is consumed later on in the for loop with xored[offset+x] = R[x] ^ xored[offset+x].
Strictly speaking, I suppose it doesn't actually need to be a standalone variable, but I felt that for readability it makes sense to follow the algorithm definition/spec, Step 6.ii - Let R = PRF(P || Q). . It's then used in the xor step in 6.iii.
In the code, R is a "sub-slice" of Y and overlaps part of PQ. These all could have been separate, but as part of performance improvements in v1.1.0, they were changed to share a backing array.
I see, a false positive (a rather odd one too). I'm all in favor of the readability aspect of intermediate variables. Thanks for setting the record straight and for such a detailed response!
It's strange that staticcheck considers it unused, because it is consumed later on
Technically, it isn't. xored[offset+x] = R[x] ^ xored[offset+x] on line 554 makes use of the value of R set on line 534: R, err = c.prf(PQ). The definition of R on line 484 is never being read.
Reopening in light of @dominikh point - it appears not to be a false positive after all.
Yeah indeed, thanks for pointing it out folks. Unfortunately I no longer have write access to this repo so TBD on when this will get fixed...