From b61a1e77af5dc458ed6a5aee395d5b22775a4917 Mon Sep 17 00:00:00 2001 From: Bohdan Date: Mon, 11 Mar 2019 15:19:55 +0200 Subject: [PATCH] Fix UB described in (#22) https://github.com/jedisct1/libsodium/issues/333#issuecomment-165375457 Signed-off-by: Bogdan Vaneev --- .gitignore | 2 ++ CMakeLists.txt | 21 +++++++----- lib/ed25519/ref10/fe_mul.c | 20 +++++------ lib/ed25519/ref10/fe_sq.c | 20 +++++------ lib/ed25519/ref10/fe_sq2.c | 20 +++++------ lib/ed25519/ref10/fe_tobytes.c | 36 ++++++++++---------- lib/ed25519/ref10/ge_scalarmult_base.c | 2 +- lib/ed25519/ref10/sc_muladd.c | 46 +++++++++++++------------- lib/ed25519/ref10/sc_reduce.c | 34 +++++++++---------- 9 files changed, 103 insertions(+), 98 deletions(-) diff --git a/.gitignore b/.gitignore index fea1788..c2639da 100644 --- a/.gitignore +++ b/.gitignore @@ -59,3 +59,5 @@ external example/build .vs +_logs +_builds diff --git a/CMakeLists.txt b/CMakeLists.txt index ab2f207..7e9db55 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,8 +10,8 @@ endif (CCACHE_FOUND) include("cmake/Hunter/init.cmake") HunterGate( - URL "https://github.com/ruslo/hunter/archive/v0.23.105.tar.gz" - SHA1 "1c7aaffc90d4898f0b55d03b77deb890b2aa0a2b" + URL "https://github.com/ruslo/hunter/archive/v0.23.127.tar.gz" + SHA1 "2cd79807cb829127896811f8afa15e790b6bde19" ) @@ -23,19 +23,22 @@ project(ed25519 VERSION ${SOVERSION} LANGUAGES C CXX) enable_language(ASM) -set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) set(CMAKE_C_VISIBILITY_PRESET hidden) set(CMAKE_CXX_VISIBILITY_PRESET hidden) set(CMAKE_VISIBILITY_INLINES_HIDDEN ON) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) -set(CMAKE_C_STANDARD 11) # force std=c11 -set(CMAKE_C_STANDARD_REQUIRED ON) -set(CMAKE_C_EXTENSIONS OFF) +if(NOT EXISTS "${CMAKE_TOOLCHAIN_FILE}") + set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) -set(CMAKE_CXX_STANDARD 11) # force std=c++11 -set(CMAKE_CXX_STANDARD_REQUIRED ON) -set(CMAKE_CXX_EXTENSIONS OFF) + set(CMAKE_C_STANDARD 11) # force std=c11 + set(CMAKE_C_STANDARD_REQUIRED ON) + set(CMAKE_C_EXTENSIONS OFF) + + set(CMAKE_CXX_STANDARD 11) # force std=c++11 + set(CMAKE_CXX_STANDARD_REQUIRED ON) + set(CMAKE_CXX_EXTENSIONS OFF) +endif() option(TESTING "Enable testing" OFF) option(COVERAGE "Enable coverage" OFF) diff --git a/lib/ed25519/ref10/fe_mul.c b/lib/ed25519/ref10/fe_mul.c index 52ae97f..4c9de55 100644 --- a/lib/ed25519/ref10/fe_mul.c +++ b/lib/ed25519/ref10/fe_mul.c @@ -179,16 +179,16 @@ void fe_mul(fe h,const fe f,const fe g) crypto_int64 h7 = f0g7+f1g6 +f2g5 +f3g4 +f4g3 +f5g2 +f6g1 +f7g0 +f8g9_19+f9g8_19; crypto_int64 h8 = f0g8+f1g7_2 +f2g6 +f3g5_2 +f4g4 +f5g3_2 +f6g2 +f7g1_2 +f8g0 +f9g9_38; crypto_int64 h9 = f0g9+f1g8 +f2g7 +f3g6 +f4g5 +f5g4 +f6g3 +f7g2 +f8g1 +f9g0 ; - crypto_int64 carry0; - crypto_int64 carry1; - crypto_int64 carry2; - crypto_int64 carry3; - crypto_int64 carry4; - crypto_int64 carry5; - crypto_int64 carry6; - crypto_int64 carry7; - crypto_int64 carry8; - crypto_int64 carry9; + crypto_uint64 carry0; + crypto_uint64 carry1; + crypto_uint64 carry2; + crypto_uint64 carry3; + crypto_uint64 carry4; + crypto_uint64 carry5; + crypto_uint64 carry6; + crypto_uint64 carry7; + crypto_uint64 carry8; + crypto_uint64 carry9; /* |h0| <= (1.65*1.65*2^52*(1+19+19+19+19)+1.65*1.65*2^50*(38+38+38+38+38)) diff --git a/lib/ed25519/ref10/fe_sq.c b/lib/ed25519/ref10/fe_sq.c index 637b101..22cc79c 100644 --- a/lib/ed25519/ref10/fe_sq.c +++ b/lib/ed25519/ref10/fe_sq.c @@ -106,16 +106,16 @@ void fe_sq(fe h,const fe f) crypto_int64 h7 = f0f7_2+f1f6_2 +f2f5_2 +f3f4_2 +f8f9_38; crypto_int64 h8 = f0f8_2+f1f7_4 +f2f6_2 +f3f5_4 +f4f4 +f9f9_38; crypto_int64 h9 = f0f9_2+f1f8_2 +f2f7_2 +f3f6_2 +f4f5_2; - crypto_int64 carry0; - crypto_int64 carry1; - crypto_int64 carry2; - crypto_int64 carry3; - crypto_int64 carry4; - crypto_int64 carry5; - crypto_int64 carry6; - crypto_int64 carry7; - crypto_int64 carry8; - crypto_int64 carry9; + crypto_uint64 carry0; + crypto_uint64 carry1; + crypto_uint64 carry2; + crypto_uint64 carry3; + crypto_uint64 carry4; + crypto_uint64 carry5; + crypto_uint64 carry6; + crypto_uint64 carry7; + crypto_uint64 carry8; + crypto_uint64 carry9; carry0 = (h0 + (crypto_int64) (1<<25)) >> 26; h1 += carry0; h0 -= carry0 << 26; carry4 = (h4 + (crypto_int64) (1<<25)) >> 26; h5 += carry4; h4 -= carry4 << 26; diff --git a/lib/ed25519/ref10/fe_sq2.c b/lib/ed25519/ref10/fe_sq2.c index 8fef841..d4600a1 100644 --- a/lib/ed25519/ref10/fe_sq2.c +++ b/lib/ed25519/ref10/fe_sq2.c @@ -106,16 +106,16 @@ void fe_sq2(fe h,const fe f) crypto_int64 h7 = f0f7_2+f1f6_2 +f2f5_2 +f3f4_2 +f8f9_38; crypto_int64 h8 = f0f8_2+f1f7_4 +f2f6_2 +f3f5_4 +f4f4 +f9f9_38; crypto_int64 h9 = f0f9_2+f1f8_2 +f2f7_2 +f3f6_2 +f4f5_2; - crypto_int64 carry0; - crypto_int64 carry1; - crypto_int64 carry2; - crypto_int64 carry3; - crypto_int64 carry4; - crypto_int64 carry5; - crypto_int64 carry6; - crypto_int64 carry7; - crypto_int64 carry8; - crypto_int64 carry9; + crypto_uint64 carry0; + crypto_uint64 carry1; + crypto_uint64 carry2; + crypto_uint64 carry3; + crypto_uint64 carry4; + crypto_uint64 carry5; + crypto_uint64 carry6; + crypto_uint64 carry7; + crypto_uint64 carry8; + crypto_uint64 carry9; h0 += h0; h1 += h1; diff --git a/lib/ed25519/ref10/fe_tobytes.c b/lib/ed25519/ref10/fe_tobytes.c index 0a63baf..c9c37fc 100644 --- a/lib/ed25519/ref10/fe_tobytes.c +++ b/lib/ed25519/ref10/fe_tobytes.c @@ -38,16 +38,16 @@ void fe_tobytes(unsigned char *s,const fe h) crypto_int32 h8 = h[8]; crypto_int32 h9 = h[9]; crypto_int32 q; - crypto_int32 carry0; - crypto_int32 carry1; - crypto_int32 carry2; - crypto_int32 carry3; - crypto_int32 carry4; - crypto_int32 carry5; - crypto_int32 carry6; - crypto_int32 carry7; - crypto_int32 carry8; - crypto_int32 carry9; + crypto_uint32 carry0; + crypto_uint32 carry1; + crypto_uint32 carry2; + crypto_uint32 carry3; + crypto_uint32 carry4; + crypto_uint32 carry5; + crypto_uint32 carry6; + crypto_uint32 carry7; + crypto_uint32 carry8; + crypto_uint32 carry9; q = (19 * h9 + (((crypto_int32) 1) << 24)) >> 25; q = (h0 + q) >> 26; @@ -87,32 +87,32 @@ void fe_tobytes(unsigned char *s,const fe h) s[0] = h0 >> 0; s[1] = h0 >> 8; s[2] = h0 >> 16; - s[3] = (h0 >> 24) | (h1 << 2); + s[3] = (h0 >> 24) | (h1 * (1u << 2u)); s[4] = h1 >> 6; s[5] = h1 >> 14; - s[6] = (h1 >> 22) | (h2 << 3); + s[6] = (h1 >> 22) | (h2 * (1u << 3u)); s[7] = h2 >> 5; s[8] = h2 >> 13; - s[9] = (h2 >> 21) | (h3 << 5); + s[9] = (h2 >> 21) | (h3 * (1u << 5u)); s[10] = h3 >> 3; s[11] = h3 >> 11; - s[12] = (h3 >> 19) | (h4 << 6); + s[12] = (h3 >> 19) | (h4 * (1u << 6u)); s[13] = h4 >> 2; s[14] = h4 >> 10; s[15] = h4 >> 18; s[16] = h5 >> 0; s[17] = h5 >> 8; s[18] = h5 >> 16; - s[19] = (h5 >> 24) | (h6 << 1); + s[19] = (h5 >> 24) | (h6 * (1u << 1u)); s[20] = h6 >> 7; s[21] = h6 >> 15; - s[22] = (h6 >> 23) | (h7 << 3); + s[22] = (h6 >> 23) | (h7 * (1u << 3u)); s[23] = h7 >> 5; s[24] = h7 >> 13; - s[25] = (h7 >> 21) | (h8 << 4); + s[25] = (h7 >> 21) | (h8 * (1u << 4u)); s[26] = h8 >> 4; s[27] = h8 >> 12; - s[28] = (h8 >> 20) | (h9 << 6); + s[28] = (h8 >> 20) | (h9 * (1u << 6u)); s[29] = h9 >> 2; s[30] = h9 >> 10; s[31] = h9 >> 18; diff --git a/lib/ed25519/ref10/ge_scalarmult_base.c b/lib/ed25519/ref10/ge_scalarmult_base.c index 142231d..d539c73 100644 --- a/lib/ed25519/ref10/ge_scalarmult_base.c +++ b/lib/ed25519/ref10/ge_scalarmult_base.c @@ -35,7 +35,7 @@ static void select(ge_precomp *t,int pos,signed char b) { ge_precomp minust; unsigned char bnegative = negative(b); - unsigned char babs = b - (((-bnegative) & b) << 1); + unsigned char babs = b - (((-bnegative) & b) * 2); ge_precomp_0(t); cmov(t,&base[pos][0],equal(babs,1)); diff --git a/lib/ed25519/ref10/sc_muladd.c b/lib/ed25519/ref10/sc_muladd.c index 99ab5e4..ef1af6a 100644 --- a/lib/ed25519/ref10/sc_muladd.c +++ b/lib/ed25519/ref10/sc_muladd.c @@ -94,29 +94,29 @@ void sc_muladd(unsigned char *s,const unsigned char *a,const unsigned char *b,co crypto_int64 s21; crypto_int64 s22; crypto_int64 s23; - crypto_int64 carry0; - crypto_int64 carry1; - crypto_int64 carry2; - crypto_int64 carry3; - crypto_int64 carry4; - crypto_int64 carry5; - crypto_int64 carry6; - crypto_int64 carry7; - crypto_int64 carry8; - crypto_int64 carry9; - crypto_int64 carry10; - crypto_int64 carry11; - crypto_int64 carry12; - crypto_int64 carry13; - crypto_int64 carry14; - crypto_int64 carry15; - crypto_int64 carry16; - crypto_int64 carry17; - crypto_int64 carry18; - crypto_int64 carry19; - crypto_int64 carry20; - crypto_int64 carry21; - crypto_int64 carry22; + crypto_uint64 carry0; + crypto_uint64 carry1; + crypto_uint64 carry2; + crypto_uint64 carry3; + crypto_uint64 carry4; + crypto_uint64 carry5; + crypto_uint64 carry6; + crypto_uint64 carry7; + crypto_uint64 carry8; + crypto_uint64 carry9; + crypto_uint64 carry10; + crypto_uint64 carry11; + crypto_uint64 carry12; + crypto_uint64 carry13; + crypto_uint64 carry14; + crypto_uint64 carry15; + crypto_uint64 carry16; + crypto_uint64 carry17; + crypto_uint64 carry18; + crypto_uint64 carry19; + crypto_uint64 carry20; + crypto_uint64 carry21; + crypto_uint64 carry22; s0 = c0 + a0*b0; s1 = c1 + a0*b1 + a1*b0; diff --git a/lib/ed25519/ref10/sc_reduce.c b/lib/ed25519/ref10/sc_reduce.c index 99c4eb0..ca6a4af 100644 --- a/lib/ed25519/ref10/sc_reduce.c +++ b/lib/ed25519/ref10/sc_reduce.c @@ -57,23 +57,23 @@ void sc_reduce(unsigned char *s) crypto_int64 s21 = 2097151 & (load_3(s + 55) >> 1); crypto_int64 s22 = 2097151 & (load_4(s + 57) >> 6); crypto_int64 s23 = (load_4(s + 60) >> 3); - crypto_int64 carry0; - crypto_int64 carry1; - crypto_int64 carry2; - crypto_int64 carry3; - crypto_int64 carry4; - crypto_int64 carry5; - crypto_int64 carry6; - crypto_int64 carry7; - crypto_int64 carry8; - crypto_int64 carry9; - crypto_int64 carry10; - crypto_int64 carry11; - crypto_int64 carry12; - crypto_int64 carry13; - crypto_int64 carry14; - crypto_int64 carry15; - crypto_int64 carry16; + crypto_uint64 carry0; + crypto_uint64 carry1; + crypto_uint64 carry2; + crypto_uint64 carry3; + crypto_uint64 carry4; + crypto_uint64 carry5; + crypto_uint64 carry6; + crypto_uint64 carry7; + crypto_uint64 carry8; + crypto_uint64 carry9; + crypto_uint64 carry10; + crypto_uint64 carry11; + crypto_uint64 carry12; + crypto_uint64 carry13; + crypto_uint64 carry14; + crypto_uint64 carry15; + crypto_uint64 carry16; s11 += s23 * 666643; s12 += s23 * 470296;