Skip to content

Commit 6aac650

Browse files
Mikaka27Mikaka27
committed
Merge branch 'michal/crypto/fix-aead-cipher-key-buffer-overread/OTP-20244' into maint
* michal/crypto/fix-aead-cipher-key-buffer-overread/OTP-20244: Fix chacha20-poly1305 key buffer overread
2 parents b6efe93 + f8d01d6 commit 6aac650

3 files changed

Lines changed: 66 additions & 11 deletions

File tree

‎lib/crypto/c_src/aead.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ ERL_NIF_TERM aead_cipher_init_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM a
134134
{ret = EXCP_ERROR(env, "Can't allocate ctx"); goto done;}
135135
if (EVP_CipherInit_ex(ctx_res->ctx, ctx_res->cipherp->cipher.p, NULL, NULL, NULL, ctx_res->encflg) != 1)
136136
{ret = EXCP_ERROR(env, "CipherInit failed"); goto done;}
137+
if (!EVP_CIPHER_CTX_set_key_length(ctx_res->ctx, (int)key.size))
138+
{ret = EXCP_BADARG_N(env, 1, "Bad Key length"); goto done;}
137139

138140
ret = enif_make_resource(env, ctx_res);
139141

@@ -233,6 +235,8 @@ ERL_NIF_TERM aead_cipher_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]
233235
{ret = EXCP_ERROR(env, "Can't allocate ctx"); goto done;}
234236
if (EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, encflg) != 1)
235237
{ret = EXCP_ERROR(env, "CipherInit failed"); goto done;}
238+
if (!EVP_CIPHER_CTX_set_key_length(ctx, (int)key.size))
239+
{ret = EXCP_BADARG_N(env, 1, "Bad Key length"); goto done;}
236240

237241
} else {
238242
/* argc = 4 {state, IV, InData, AAD } */

‎lib/crypto/c_src/cipher.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ static struct cipher_type_t cipher_types[] =
127127

128128
/*==== AEAD ciphers ====*/
129129
#if defined(HAVE_CHACHA20_POLY1305)
130-
{{"chacha20_poly1305"}, "chacha20-poly1305", {&EVP_chacha20_poly1305}, 0, NO_FIPS_CIPHER | AEAD_CIPHER, AEAD_CTRL},
130+
{{"chacha20_poly1305"}, "chacha20-poly1305", {&EVP_chacha20_poly1305}, 32, NO_FIPS_CIPHER | AEAD_CIPHER, AEAD_CTRL},
131131
#else
132132
{{"chacha20_poly1305"}, "chacha20-poly1305", {NULL}, 0, NO_FIPS_CIPHER | AEAD_CIPHER, {{0,0,0}}},
133133
#endif

‎lib/crypto/test/crypto_SUITE.erl‎

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@
3737
end_per_testcase/2,
3838

3939
%% Test cases:
40-
aead_bad_tag/1,
4140
aead_ng/1,
41+
aead_bad_tag/1,
42+
aead_bad_key_length/1,
4243
all_ciphers/1,
4344
api_errors_ecdh/1,
4445
api_errors_aead/1,
@@ -477,9 +478,9 @@ groups() ->
477478
{sm4_ofb, [], [api_ng, api_ng_one_shot]},
478479
{sm4_cfb, [], [api_ng, api_ng_one_shot]},
479480
{sm4_ctr, [], [api_ng, api_ng_one_shot]},
480-
{sm4_gcm, [], [aead_ng, aead_bad_tag]},
481-
{sm4_ccm, [], [aead_ng, aead_bad_tag]},
482-
{chacha20_poly1305, [], [aead_ng, aead_bad_tag]},
481+
{sm4_gcm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
482+
{sm4_ccm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
483+
{chacha20_poly1305, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
483484
{chacha20, [], [api_ng, api_ng_one_shot]},
484485
{poly1305, [], [poly1305]},
485486
{no_poly1305, [], [no_poly1305]},
@@ -526,15 +527,15 @@ groups() ->
526527
{aes_128_ctr, [], [api_ng, api_ng_one_shot]},
527528
{aes_192_ctr, [], [api_ng, api_ng_one_shot]},
528529
{aes_256_ctr, [], [api_ng, api_ng_one_shot]},
529-
{aes_128_ccm, [], [aead_ng, aead_bad_tag]},
530-
{aes_192_ccm, [], [aead_ng, aead_bad_tag]},
531-
{aes_256_ccm, [], [aead_ng, aead_bad_tag]},
530+
{aes_128_ccm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
531+
{aes_192_ccm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
532+
{aes_256_ccm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
532533
{aes_128_ecb, [], [api_ng, api_ng_one_shot]},
533534
{aes_192_ecb, [], [api_ng, api_ng_one_shot]},
534535
{aes_256_ecb, [], [api_ng, api_ng_one_shot]},
535-
{aes_128_gcm, [], [aead_ng, aead_bad_tag]},
536-
{aes_192_gcm, [], [aead_ng, aead_bad_tag]},
537-
{aes_256_gcm, [], [aead_ng, aead_bad_tag]},
536+
{aes_128_gcm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
537+
{aes_192_gcm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
538+
{aes_256_gcm, [], [aead_ng, aead_bad_tag, aead_bad_key_length]},
538539
{aes_128_ofb, [], [api_ng, api_ng_one_shot]},
539540
{aes_192_ofb, [], [api_ng, api_ng_one_shot]},
540541
{aes_256_ofb, [], [api_ng, api_ng_one_shot]}
@@ -1211,6 +1212,23 @@ aead_bad_tag(Config) ->
12111212
end,
12121213
do_cipher_tests(fun aead_cipher_bad_tag/1, FilteredAEADs).
12131214

1215+
%%--------------------------------------------------------------------
1216+
aead_bad_key_length(Config) ->
1217+
[_|_] = AEADs = lazy_eval(proplists:get_value(cipher, Config)),
1218+
FilteredAEADs =
1219+
case proplists:get_bool(fips, Config) of
1220+
false ->
1221+
AEADs;
1222+
true ->
1223+
%% In FIPS mode, the IV length must be at least 12 bytes.
1224+
lists:filter(
1225+
fun(Tuple) ->
1226+
IVLen = byte_size(element(4, Tuple)),
1227+
IVLen >= 12
1228+
end, AEADs)
1229+
end,
1230+
do_cipher_tests(fun aead_cipher_bad_key_length/1, FilteredAEADs).
1231+
12141232
%%--------------------------------------------------------------------
12151233
sign_verify() ->
12161234
[{doc, "Sign/verify digital signatures"}].
@@ -1909,6 +1927,39 @@ aead_cipher_bad_tag({Type, Key, _PlainText, IV, AAD, CipherText, CipherTag, TagL
19091927
fun() -> crypto:crypto_one_time_aead(Type, Key, IV, CipherText, AAD, BadTruncatedTag, false) end,
19101928
error).
19111929

1930+
aead_cipher_bad_key_length({Type, Key, PlainText, IV, AAD, _CipherText, _CipherTag, _Info}) ->
1931+
F = fun(K) ->
1932+
try crypto:crypto_one_time_aead(Type, K, IV, PlainText, AAD, true) of
1933+
Res1 -> ct:fail("Call should fail, but succeeded with return: ~p~n", [Res1])
1934+
catch error : {badarg, _, _} -> ok
1935+
end,
1936+
try crypto:crypto_one_time_aead_init(Type, K, 1, true) of
1937+
Res2 -> ct:fail("Call should fail, but succeeded with return: ~p~n", [Res2])
1938+
catch error : {badarg, _, _} -> ok
1939+
end
1940+
end,
1941+
KeyExtended = <<Key/binary, 1>>,
1942+
F(KeyExtended),
1943+
KeyTruncatedSize = byte_size(Key) - 1,
1944+
<<KeyTruncated:KeyTruncatedSize/binary, _/binary>> = Key,
1945+
F(KeyTruncated);
1946+
aead_cipher_bad_key_length({Type, Key, PlainText, IV, AAD, _CipherText, _CipherTag, TagLen, _Info}) ->
1947+
F = fun(K) ->
1948+
try crypto:crypto_one_time_aead(Type, K, IV, PlainText, AAD, TagLen, true) of
1949+
Res1 -> ct:fail("Call should fail, but succeeded with return: ~p~n", [Res1])
1950+
catch error : {badarg, _, _} -> ok
1951+
end,
1952+
try crypto:crypto_one_time_aead_init(Type, K, TagLen, true) of
1953+
Res2 -> ct:fail("Call should fail, but succeeded with return: ~p~n", [Res2])
1954+
catch error : {badarg, _, _} -> ok
1955+
end
1956+
end,
1957+
KeyExtended = <<Key/binary, 1>>,
1958+
F(KeyExtended),
1959+
KeyTruncatedSize = byte_size(Key) - 1,
1960+
<<KeyTruncated:KeyTruncatedSize/binary, _/binary>> = Key,
1961+
F(KeyTruncated).
1962+
19121963

19131964
cipher_test(T, Fe, Ee, Fd, Ed) ->
19141965
%% Test encrypt

0 commit comments

Comments
 (0)