From: Mark M. <Mar...@ij...> - 2006-11-10 15:28:44
|
Noticing that selftest on installing Mail::DKIM 0.20 causes perl to segfault while checking the last signature in multiple_1.txt (test t/verifier.t), I recompiled RSA.xs with debugging and checked what is going on: Program received signal SIGSEGV, Segmentation fault. (gdb) bt #0 0x2825f78e in memmove () from /lib/libc.so.6 #1 0x280f53ae in Perl_sv_setpvn () from .../5.8.8/mach/CORE/libperl.so #2 0x280f7859 in Perl_newSVpv () from .../5.8.8/mach/CORE/libperl.so #3 0x283964ff in rsa_crypt (p_rsa=0x8561164, p_from=0x849238c, p_crypt=0x28332a10 <RSA_public_encrypt>) at RSA.xs:203 #4 0x28397ddb in XS_Crypt__OpenSSL__RSA_encrypt (cv=0x849196c) at RSA.xs:404 #5 0x280f01c0 in Perl_pp_entersub () from .../5.8.8/mach/CORE/libperl.so #6 0x280e91a5 in Perl_runops_standard () from .../5.8.8/mach/CORE/libperl.so #7 0x28098b8a in perl_run () from .../5.8.8/mach/CORE/libperl.so #8 0x08048fe2 in main () (gdb) up #1 0x280f53ae in Perl_sv_setpvn () from .../5.8.8/mach/CORE/libperl.so (gdb) up #2 0x280f7859 in Perl_newSVpv () from .../5.8.8/mach/CORE/libperl.so (gdb) up #3 0x283964ff in rsa_crypt (p_rsa=0x8561164, p_from=0x849238c, p_crypt=0x28332a10 <RSA_public_encrypt>) at RSA.xs:203 203 sv = newSVpv(to, to_length); (gdb) p to_length $2 = 4294967295 The code in RSA.xs in question is: SV* rsa_crypt(rsaData* p_rsa, SV* p_from, int (*p_crypt)(int, const unsigned char*, unsigned char*, RSA*, int)) { STRLEN from_length, to_length; ... to_length = p_crypt( from_length, from, (unsigned char*) to, p_rsa->rsa, p_rsa->padding); if (to_length < 0) <-- is false because to_length is unsigned { Safefree(to); CHECK_OPEN_SSL(0); } --> crashes here: sv = newSVpv(to, to_length); So the problem is that p_crypt returns -1, which is converted to unsigned 4294967295, and memmove crashes trying to copy 4 GB of nonexistent data to some allocated buffer. I'd be surprised if there is no security problem, although at the moment I'm more concerned with keeping Perl application programs stable. There are other potentially troublesome signed/unsigned conversions in RSA.xs and typecasting 64-bit pointers to smaller integers, although I haven't examined them closely. Btw, STRLEN boils down to size_t, which is unsigned 32-bit value. This is on FreeBSD 6.1, Perl 5.8.8 from ports, security/p5-Crypt-OpenSSL-RSA 0.23 from ports, although this probably applies to most modern platforms. Regards Mark |
From: Ian R. <ia...@re...> - 2006-11-12 20:48:50
|
Mark, Thanks for the bug report. I've checked in a change to make to_length an int, instead of STRLEN. However, before cutting a release, I'd like if possible to understand the core problem. Would you mind editing your copy of RSA.xs to change to_length to an int, and then see what error is getting reported by the call to CHECK_OPEN_SSL(0)? - Ian On Fri, 2006-11-10 at 16:28 +0100, Mark Martinec wrote: > Noticing that selftest on installing Mail::DKIM 0.20 causes perl > to segfault while checking the last signature in multiple_1.txt > (test t/verifier.t), I recompiled RSA.xs with debugging and > checked what is going on: > > Program received signal SIGSEGV, Segmentation fault. > > (gdb) bt > #0 0x2825f78e in memmove () from /lib/libc.so.6 > #1 0x280f53ae in Perl_sv_setpvn () from .../5.8.8/mach/CORE/libperl.so > #2 0x280f7859 in Perl_newSVpv () from .../5.8.8/mach/CORE/libperl.so > #3 0x283964ff in rsa_crypt (p_rsa=0x8561164, p_from=0x849238c, > p_crypt=0x28332a10 <RSA_public_encrypt>) at RSA.xs:203 > #4 0x28397ddb in XS_Crypt__OpenSSL__RSA_encrypt (cv=0x849196c) at RSA.xs:404 > #5 0x280f01c0 in Perl_pp_entersub () from .../5.8.8/mach/CORE/libperl.so > #6 0x280e91a5 in Perl_runops_standard () from .../5.8.8/mach/CORE/libperl.so > #7 0x28098b8a in perl_run () from .../5.8.8/mach/CORE/libperl.so > #8 0x08048fe2 in main () > > (gdb) up > #1 0x280f53ae in Perl_sv_setpvn () from .../5.8.8/mach/CORE/libperl.so > (gdb) up > #2 0x280f7859 in Perl_newSVpv () from .../5.8.8/mach/CORE/libperl.so > (gdb) up > #3 0x283964ff in rsa_crypt (p_rsa=0x8561164, p_from=0x849238c, > p_crypt=0x28332a10 <RSA_public_encrypt>) at RSA.xs:203 > 203 sv = newSVpv(to, to_length); > > (gdb) p to_length > $2 = 4294967295 > > The code in RSA.xs in question is: > > SV* rsa_crypt(rsaData* p_rsa, SV* p_from, > int (*p_crypt)(int, const unsigned char*, unsigned char*, RSA*, int)) > { > STRLEN from_length, to_length; > ... > to_length = p_crypt( > from_length, from, (unsigned char*) to, p_rsa->rsa, p_rsa->padding); > > if (to_length < 0) <-- is false because to_length is unsigned > { > Safefree(to); > CHECK_OPEN_SSL(0); > } > --> crashes here: > sv = newSVpv(to, to_length); > > So the problem is that p_crypt returns -1, which is converted > to unsigned 4294967295, and memmove crashes trying to copy > 4 GB of nonexistent data to some allocated buffer. > > I'd be surprised if there is no security problem, although at the > moment I'm more concerned with keeping Perl application programs stable. > There are other potentially troublesome signed/unsigned conversions > in RSA.xs and typecasting 64-bit pointers to smaller integers, > although I haven't examined them closely. > > Btw, STRLEN boils down to size_t, which is unsigned 32-bit > value. This is on FreeBSD 6.1, Perl 5.8.8 from ports, > security/p5-Crypt-OpenSSL-RSA 0.23 from ports, although this > probably applies to most modern platforms. > > Regards > Mark > > ------------------------------------------------------------------------- > Using Tomcat but need to do more? Need to support web services, security? > Get stuff done quickly with pre-integrated technology to make your job easier > Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 > _______________________________________________ > Perl-openssl-users mailing list > Per...@li... > https://lists.sourceforge.net/lists/listinfo/perl-openssl-users |
From: Mark M. <Mar...@ij...> - 2006-11-13 01:31:47
|
On Sunday November 12 2006 21:48, Ian Robertson wrote: > Thanks for the bug report. I've checked in a change to make to_length > an int, instead of STRLEN. However, before cutting a release, I'd like > if possible to understand the core problem. Would you mind editing your > copy of RSA.xs to change to_length to an int, and then see what error is > getting reported by the call to CHECK_OPEN_SSL(0)? Thanks Ian, this now reveals the condition that lead to the failure: $ Mail-DKIM-0.20/t/verifier.t ... # verifying message 'multiple_1.txt' DEBUG-OUTPUT: RSA.xs:201: OpenSSL error: data too small for key size at /usr/local/lib/perl5/site_perl/5.8.8/Mail/DKIM/PublicKey.pm line 305. # result: pass It is an application error, in this case probably intentional, as it is part of installation tests for module Mail::DKIM, trying all sorts of valid an invalid keys. It certainly had no right to crash the process. Thanks for looking into it! A note to Jason Long, the author of Mail::DKIM : the DEBUG-OUTPUT line above was produced by a print I inserted into Mail::DKIM::Verifier::finish_body, after desperately trying to obtain the error report. It turned out that the "local $@" within "try" in sub finish_body should not be there, as it prevents the error report in $@ to reach outer scope and the $E in "otherwise" block receives a useless error string. Regards Mark |