@@ -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)
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@outlook.com> --- libavutil/aes.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)