ERL-673: Segmentation fault in crypto module

[OTP-Maintainer]

Original reporter: dansarie Affected version: OTP-20.0 Fixed in versions: OTP-20.3.8.5, OTP-21.0.5 Component: crypto Migrated from: https://bugs.erlang.org/browse/ERL-673

---

Using any term other than an integer or binary as the second argument of crypto:compute_key/4 (OtherPublicKey) crashes the Erlang VM.

Steps to reproduce:
{code}
Erlang/OTP 20 [erts-9.2] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:10] [kernel-poll:false]

Eshell V9.2  (abort with ^G)
1> {Pub, Priv} = crypto:generate_key(ecdh, sect571r1).
{<<4,2,235,95,224,176,123,92,220,161,19,218,150,90,105,73,
   130,110,248,235,161,13,140,173,162,48,86,47,...>>,
 <<1,149,117,187,93,141,95,67,206,212,247,93,164,200,38,
   130,168,242,220,159,232,171,113,97,107,112,168,...>>}
2> crypto:compute_key(ecdh, foo, Priv, sect571r1).
Segmentation fault (core dumped)
{code}

The atom foo can be replaced by any arbitrary non-integer and non-binary term with the same result.

A cursory inspection of the source (crypto.erl) indicates that adding an is_binary guard to ensure_int_as_bin/1 would be a quick fix for the problem. The core issue, however, is in the ecdh_compute_key_nif native function or one of the functions it calls.

[OTP-Maintainer]

starbelly said:

Replicated:

{code:erlang}
Erlang/OTP 20 [erts-9.3] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V9.3  (abort with ^G)
1> {Pub, Priv} = crypto:generate_key(ecdh, sect571r1).
{<<4,1,69,207,99,222,75,204,134,124,249,67,211,6,52,208,
   201,166,46,149,17,221,231,44,98,22,187,54,...>>,
 <<1,180,142,21,215,12,27,220,125,219,142,79,122,78,15,60,
   81,163,167,133,141,126,255,87,11,18,228,...>>}
2> crypto:compute_key(ecdh, foo, Priv, sect571r1).
fish: 'erl' terminated by signal SIGSEGV (Address boundary error)
{code}

On OTP 21 this results in a badarg (correct behavior):

{code:erlang}
Erlang/OTP 21 [erts-10.0.3] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [hipe]

Eshell V10.0.3  (abort with ^G)
1> {Pub, Priv} = crypto:generate_key(ecdh, sect571r1).
{<<4,5,116,214,144,104,20,51,1,106,16,145,77,91,212,66,
   156,32,122,78,209,101,143,93,134,216,97,254,...>>,
 <<1,193,33,85,126,89,64,96,152,8,247,232,194,233,55,23,
   149,232,216,183,110,176,49,131,187,39,88,...>>}
2> crypto:compute_key(ecdh, foo, Priv, sect571r1).
** exception error: bad argument
     in function  crypto:ecdh_compute_key_nif/3
        called as crypto:ecdh_compute_key_nif(foo,
                                              {{characteristic_two_field,571,{ppbasis,2,5,10}},
                                               {<<1>>,
                                                <<2,244,14,126,34,33,242,149,222,41,113,23,183,243,214,47,
                                                  92,106,151,255,203,140,239,...>>,
                                                <<42,160,88,247,58,14,51,171,72,107,15,97,4,16,197,58,
                                                  127,19,35,16>>},
                                               <<4,3,3,0,29,52,184,86,41,108,22,192,212,13,60,215,117,10,
                                                 147,209,210,149,95,168,...>>,
                                               <<3,255,255,255,255,255,255,255,255,255,255,255,255,255,
                                                 255,255,255,255,255,255,255,255,255,...>>,
                                               <<2>>},
                                              <<1,193,33,85,126,89,64,96,152,8,247,232,194,233,55,23,
                                                149,232,216,183,110,176,49,131,187,39,...>>)
3>
{code}
I am looking into a patch...

[OTP-Maintainer]

starbelly said:

I can reproduce this on maint-20 if HiPE is enabled on Mac Os High sierra,  Will update this if I can gather more information. This is _not_ reproducible if HiPE is disabled. I do not have a patch for this. 

[OTP-Maintainer]

zxq9 said:

Still happens on 20.3 on Linux/Debian (built with Kerl):

{code}
ceverett@takoyaki:~/vcs/zomp/src$ erl
Erlang/OTP 20 [erts-9.3] [source] [64-bit] [smp:2:2] [ds:2:2:10] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V9.3  (abort with ^G)
1> {Pub, Priv} = crypto:generate_key(ecdh, sect571r1).
{<<4,3,34,56,201,6,144,31,8,202,34,13,45,125,217,86,103,
  191,85,142,85,194,95,55,39,119,125,27,...>>,
<<1,39,183,189,215,74,140,221,79,38,15,236,136,162,98,50,
  155,204,117,53,95,136,131,192,86,61,138,...>>}
2> crypto:compute_key(ecdh, foo, Priv, sect571r1).
Segmentation fault (コアダンプ)
{code}

[OTP-Maintainer]

starbelly said:

A PR which adds guards to compute_get/4 (ecdh) as a preventative measure : https://github.com/erlang/otp/pull/1879

[OTP-Maintainer]

beardedeagle said:

I tried uploading images, but it didn't work out so well. Either way, When running the following code on Windows Server 2016 with OTP 20.3, it causes erl to crash:

{code:erlang}
{Pub, Priv} = crypto:generate_key(ecdh, sect571r1).
crypto:compute_key(ecdh, foo, Priv, sect571r1).
{code}

[OTP-Maintainer]

starbelly said:

Second attempt at a PR - https://github.com/erlang/otp/pull/1884

[OTP-Maintainer]

john said:

bq. Second attempt at a PR - https://github.com/erlang/otp/pull/1884

I could've sworn I posted a link to a PR fixing this yesterday, but I must've been distracted. Sorry about the confusion. :(

https://github.com/erlang/otp/pull/1880

[OTP-Maintainer]

starbelly said:

John... Absolutely no problem! I believe I was in the wrong and you did indeed add a link... I just didn't slow down enough to see it :D I was happy enough to have dove head first into crypto.c and come up with a fix on my own :) I also very much appreciate your patience and feedback, mentoring is everything! Until next time ;)