Skip to content

Commit 8c68d99

Browse files
cyanogilvieclaude
andcommitted
Fix s_ocb_done aliasing bug in decrypt path
In decrypt mode (mode==1), s_ocb_done was XORing `ct[x]` into the checksum before writing the output. The function's parameter names are misleading (the header comment notes pt/ct really mean in/out), so in decrypt mode `ct` is the not-yet-written *output* buffer and `pt` is the *input* ciphertext. Reading from `ct` only worked when callers aliased the input and output buffers (in-place decryption), as the self-test does. Callers passing distinct buffers got CRYPT_OK with stat=0 -- correct plaintext but failed tag verification. Fix by reading from `pt[x]` (the input). Add a separate-buffer regression case to ocb_test that runs against every existing test vector and was confirmed to fail without the fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e5bfb94 commit 8c68d99

File tree

2 files changed

+31
-4
lines changed

2 files changed

+31
-4
lines changed

src/encauth/ocb/ocb_test.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ int ocb_test(void)
158158

159159
int err, x, idx, res;
160160
unsigned long len;
161-
unsigned char outct[MAXBLOCKSIZE], outtag[MAXBLOCKSIZE];
161+
unsigned char outct[MAXBLOCKSIZE], outtag[MAXBLOCKSIZE], outpt[MAXBLOCKSIZE];
162162

163163
/* AES can be under rijndael or aes... try to find it */
164164
if ((idx = find_cipher("aes")) == -1) {
@@ -179,6 +179,25 @@ int ocb_test(void)
179179
return CRYPT_FAIL_TESTVECTOR;
180180
}
181181

182+
/* Decrypt with separate input and output buffers. Historically
183+
* s_ocb_done() had an aliasing bug in its decrypt path that only
184+
* surfaced when ct and pt were distinct buffers (the earlier
185+
* in-place call below masked it). Run this case first so it is
186+
* exercised on every test vector.
187+
*/
188+
XMEMSET(outpt, 0, sizeof(outpt));
189+
if ((err = ocb_decrypt_verify_memory(idx, tests[x].key, 16, tests[x].nonce, outct, tests[x].ptlen,
190+
outpt, tests[x].tag, len, &res)) != CRYPT_OK) {
191+
return err;
192+
}
193+
if ((res != 1) || ltc_compare_testvector(outpt, tests[x].ptlen, tests[x].pt, tests[x].ptlen, "OCB separate-buffer", x)) {
194+
#ifdef LTC_TEST_DBG
195+
printf("\n\nOCB: Failure-decrypt (separate buffers) - res = %d\n", res);
196+
#endif
197+
return CRYPT_FAIL_TESTVECTOR;
198+
}
199+
200+
/* Also exercise the in-place form for backward compatibility. */
182201
if ((err = ocb_decrypt_verify_memory(idx, tests[x].key, 16, tests[x].nonce, outct, tests[x].ptlen,
183202
outct, tests[x].tag, len, &res)) != CRYPT_OK) {
184203
return err;

src/encauth/ocb/s_ocb_done.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,18 @@ int s_ocb_done(ocb_state *ocb, const unsigned char *pt, unsigned long ptlen,
7777
}
7878

7979
if (mode == 1) {
80-
/* decrypt mode, so let's xor it first */
81-
/* xor C[m] into checksum */
80+
/* decrypt mode: xor C[m] into checksum. The function's parameter
81+
* names are misleading (see header comment) -- in decrypt mode the
82+
* input ciphertext lives in `pt` (not `ct`), and `ct` is the output
83+
* plaintext buffer that has not been written yet. Reading from `ct`
84+
* here only happens to work when the caller aliases the input and
85+
* output buffers (in-place decryption); with separate buffers the
86+
* checksum is computed against uninitialised memory and the tag
87+
* verification fails. Use `pt` (the input parameter) so the code
88+
* works for both in-place and separate-buffer callers.
89+
*/
8290
for (x = 0; x < (int)ptlen; x++) {
83-
ocb->checksum[x] ^= ct[x];
91+
ocb->checksum[x] ^= pt[x];
8492
}
8593
}
8694

0 commit comments

Comments
 (0)