While reading src/l1/a5.c I noticed that gmr1_a5(n, ...) silently
falls through its switch default for n >= 2, leaving the caller's
dl/ul buffers untouched. The only in-tree caller, at
src/gmr1_rx.c:518 (TCH3 decode), passes n as a data-driven value
(cd->tch3_state.ciph) into an uninitialized stack buffer
ciph[208], which is then fed to gmr1_tch3_decode() as a keystream.
For any n outside {0, 1} this means uninitialized stack memory
becomes the "keystream" that the decoder XORs against the burst.
Patch 1/2 is the defensive fix: zero the output buffers in the
default case, matching the n=0 "no ciphering" path so the
function's postcondition (buffers are fully written whenever
non-NULL) holds for every legal value of n. No algorithmic
change to gmr1_a5_1 or the n=0 / n=1 paths.
Patch 2/2 is cosmetic: it clarifies the Doxygen block on
gmr1_a5() to document the accepted range of n and the behavior
per case, and it adds a \see reference on gmr1_a5_1() to the
public algorithm description by Driessen, Hund, Willems, Paar,
Holz ("Don't Trust Satellite Phones", IEEE S&P 2012) which
underpins the existing implementation credit.
Both patches apply cleanly on top of master (2b98363, "avoid
memory leaks in gsmtap error paths").
Dominik Bay (2):
l1/a5: zero output buffers for unsupported A5 variants
l1/a5: expand dispatcher doc and cite upstream reference
src/l1/a5.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
--
2.50.1 (Apple Git-155)
