[FFmpeg-devel] [PATCH 4/7] avutil/aes: Don't use misaligned pointers

[Andreas Rheinhardt]

The AES code uses av_aes_block, a union consisting of
uint64_t[2], uint32_t[4], uint8_t[4][4] and uint8_t[16].
subshift() performs byte-wise manipulations of two av_aes_blocks,
but when encrypting, it does so with a shift of two bytes;
more precisely, it uses
"av_aes_block *s1 = (av_aes_block *) (s0[0].u8 - s)"
and lateron uses the uint8_t[16] member to access s0.
Yet av_aes_block requires to be suitably aligned for
the uint64_t[2] member, which s0[0].u8 - 2 is certainly
not. This is in violation of 6.3.2.3 (7) of C11. UBSan
reports this in the aes_ctr, mov-3elist-encrypted,
mov-frag-encrypted, mov-tenc-only-encrypted and srtp
tests.
Furthermore, there is another issue here: The pointer points
outside of s0; this works, because all the accesses lateron
use an index >= 3. (Clang-)UBSan reports this as
"runtime error: index -2 out of bounds for type 'uint8_t[16]'".

This commit fixes both of these issues: The latter issue
is fixed by applying an offset of "+ 3" during the cast
and subtracting this from the indices used lateron.
The former issue is solved by not casting to av_aes_block*
at all; instead simply cast to unsigned char*.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
---
 libavutil/aes.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/libavutil/aes.c b/libavutil/aes.c
index 029d738f87..8b78daa782 100644
--- a/libavutil/aes.c
+++ b/libavutil/aes.c
@@ -80,25 +80,27 @@ static inline void addkey_d(uint8_t *dst, const av_aes_block *src,
 
 static void subshift(av_aes_block s0[2], int s, const uint8_t *box)
 {
-    av_aes_block *s1 = (av_aes_block *) (s0[0].u8 - s);
-    av_aes_block *s3 = (av_aes_block *) (s0[0].u8 + s);
+    unsigned char *s1_dst = (unsigned char*)s0[0].u8 + 3 - s;
+    const unsigned char *s1_src = s1_dst + sizeof(*s0);
+    unsigned char *s3_dst = (unsigned char*)s0[0].u8 + s + 1;
+    const unsigned char *s3_src = s3_dst + sizeof(*s0);
 
     s0[0].u8[ 0] = box[s0[1].u8[ 0]];
     s0[0].u8[ 4] = box[s0[1].u8[ 4]];
     s0[0].u8[ 8] = box[s0[1].u8[ 8]];
     s0[0].u8[12] = box[s0[1].u8[12]];
-    s1[0].u8[ 3] = box[s1[1].u8[ 7]];
-    s1[0].u8[ 7] = box[s1[1].u8[11]];
-    s1[0].u8[11] = box[s1[1].u8[15]];
-    s1[0].u8[15] = box[s1[1].u8[ 3]];
+    s1_dst[ 0] = box[s1_src[ 4]];
+    s1_dst[ 4] = box[s1_src[ 8]];
+    s1_dst[ 8] = box[s1_src[12]];
+    s1_dst[12] = box[s1_src[ 0]];
     s0[0].u8[ 2] = box[s0[1].u8[10]];
     s0[0].u8[10] = box[s0[1].u8[ 2]];
     s0[0].u8[ 6] = box[s0[1].u8[14]];
     s0[0].u8[14] = box[s0[1].u8[ 6]];
-    s3[0].u8[ 1] = box[s3[1].u8[13]];
-    s3[0].u8[13] = box[s3[1].u8[ 9]];
-    s3[0].u8[ 9] = box[s3[1].u8[ 5]];
-    s3[0].u8[ 5] = box[s3[1].u8[ 1]];
+    s3_dst[ 0] = box[s3_src[12]];
+    s3_dst[12] = box[s3_src[ 8]];
+    s3_dst[ 8] = box[s3_src[ 4]];
+    s3_dst[ 4] = box[s3_src[ 0]];
 }
 
 static inline int mix_core(uint32_t multbl[][256], int a, int b, int c, int d)
-- 
2.34.1