diff mbox

milenage/aes: Address undefined behavior on bitshift

Message ID 1461490813-5032-1-git-send-email-holger@freyther.de
State New
Headers show

Commit Message

Holger Freyther April 24, 2016, 9:40 a.m. UTC
From: Holger Hans Peter Freyther <holger@moiji-mobile.com>

Extend the u8 to u32 before going to shift it.

Fixes:
milenage/aes-internal.c:799:4: runtime error: left shift of 128 by 24 places cannot be represented in type 'int'
    #0 0x7f84e9fe86a2 in rijndaelKeySetupEnc (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xfa6a2)
    #1 0x7f84e9febad8 in aes_encrypt_init (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xfdad8)
    #2 0x7f84e9fe7d14 in aes_128_encrypt_block (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xf9d14)
    #3 0x7f84e9febe7d in milenage_f1 (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xfde7d)
    #4 0x7f84e9fee2ce in milenage_generate (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0x1002ce)
    #5 0x7f84e9fe76d7 in milenage_gen_vec (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xf96d7)
    #6 0x7f84e9fe6c08 in osmo_auth_gen_vec (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xf8c08)
    #7 0x401441 in main (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/tests/auth/.libs/lt-milenage_test+0x401441)
    #8 0x7f84e8e33a3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x20a3f)
    #9 0x400e58 in _start (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/tests/auth/.libs/lt-milenage_test+0x400e58)
---
 src/gsm/milenage/aes_i.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Max April 24, 2016, 9:46 a.m. UTC | #1
I would feel more comfortable to just link (optionally) to external
crypto library which is actively maintained (nettle? libgcrypt?) rather
than fixing copy-pasted code.

On 04/24/2016 11:40 AM, Holger Hans Peter Freyther wrote:
> From: Holger Hans Peter Freyther <holger@moiji-mobile.com>
>
> Extend the u8 to u32 before going to shift it.
>
> Fixes:
> milenage/aes-internal.c:799:4: runtime error: left shift of 128 by 24 places cannot be represented in type 'int'
>     #0 0x7f84e9fe86a2 in rijndaelKeySetupEnc (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xfa6a2)
>     #1 0x7f84e9febad8 in aes_encrypt_init (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xfdad8)
>     #2 0x7f84e9fe7d14 in aes_128_encrypt_block (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xf9d14)
>     #3 0x7f84e9febe7d in milenage_f1 (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xfde7d)
>     #4 0x7f84e9fee2ce in milenage_generate (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0x1002ce)
>     #5 0x7f84e9fe76d7 in milenage_gen_vec (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xf96d7)
>     #6 0x7f84e9fe6c08 in osmo_auth_gen_vec (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/src/gsm/.libs/libosmogsm.so.5+0xf8c08)
>     #7 0x401441 in main (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/tests/auth/.libs/lt-milenage_test+0x401441)
>     #8 0x7f84e8e33a3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x20a3f)
>     #9 0x400e58 in _start (/home/builder/jenkins/workspace/Osmocom_Sanitizer/source/libosmocore/tests/auth/.libs/lt-milenage_test+0x400e58)
> ---
>  src/gsm/milenage/aes_i.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gsm/milenage/aes_i.h b/src/gsm/milenage/aes_i.h
> index c831757..5d89abc 100644
> --- a/src/gsm/milenage/aes_i.h
> +++ b/src/gsm/milenage/aes_i.h
> @@ -66,7 +66,7 @@ extern const u8 rcons[10];
>  
>  #else /* AES_SMALL_TABLES */
>  
> -#define RCON(i) (rcons[(i)] << 24)
> +#define RCON(i) ((u32)rcons[(i)] << 24)
>  
>  static inline u32 rotr(u32 val, int bits)
>  {
diff mbox

Patch

diff --git a/src/gsm/milenage/aes_i.h b/src/gsm/milenage/aes_i.h
index c831757..5d89abc 100644
--- a/src/gsm/milenage/aes_i.h
+++ b/src/gsm/milenage/aes_i.h
@@ -66,7 +66,7 @@  extern const u8 rcons[10];
 
 #else /* AES_SMALL_TABLES */
 
-#define RCON(i) (rcons[(i)] << 24)
+#define RCON(i) ((u32)rcons[(i)] << 24)
 
 static inline u32 rotr(u32 val, int bits)
 {