Le 11/09/2020 à 21:38, Salvatore Bonaccorso a écrit : > Hi Xavier, > > On Fri, Sep 11, 2020 at 06:02:00PM +0200, Xavier Guimard wrote: >> Package: release.debian.org >> Severity: normal >> Tags: buster >> User: release.debian....@packages.debian.org >> Usertags: pu >> X-Debbugs-Cc: debian-p...@lists.debian.org >> >> [ Reason ] >> libdbi-perl is vulnerable to (low) security bug (CVE-2020-14392) >> >> [ Impact ] >> libdbi-perl may crash if an attacker can give a malformed login >> >> [ Tests ] >> No new test, current passed >> >> [ Risks ] >> This patch is very simple >> >> [ Checklist ] >> [X] *all* changes are documented in the d/changelog >> [X] I reviewed all changes and I approve them >> [X] attach debdiff against the package in (old)stable >> [X] the issue is verified as fixed in unstable >> >> [ Changes ] >> Returned values are more tested > >> diff --git a/debian/changelog b/debian/changelog >> index d2e35cc..d0ad39a 100644 >> --- a/debian/changelog >> +++ b/debian/changelog >> @@ -1,3 +1,10 @@ >> +libdbi-perl (1.642-1+deb10u1) buster; urgency=medium >> + >> + * Fix memory corruption in XS functions when Perl stack is reallocated >> + (Closes: CVE-2020-14392) > > Note that there is as well CVE-2020-14393, could you add the fix for > this one as well?
Thanks for pointing this, here is a new debdiff
diff --git a/debian/changelog b/debian/changelog index d2e35cc..7b26f2f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +libdbi-perl (1.642-1+deb10u1) buster; urgency=medium + + * Fix memory corruption in XS functions when Perl stack is reallocated + (Closes: CVE-2020-14392) + * Fix a buffer overlfow on an overlong DBD class name + (Closes: CVE-2020-14393) + + -- Xavier Guimard <y...@debian.org> Thu, 10 Sep 2020 10:04:13 +0200 + libdbi-perl (1.642-1) unstable; urgency=medium [ Xavier Guimard ] diff --git a/debian/patches/CVE-2020-14392.patch b/debian/patches/CVE-2020-14392.patch new file mode 100644 index 0000000..99c7a3e --- /dev/null +++ b/debian/patches/CVE-2020-14392.patch @@ -0,0 +1,318 @@ +Description: Fix memory corruption in XS functions when Perl stack is reallocated + Macro ST(*) returns pointer to Perl stack. Other Perl functions which use + Perl stack (e.g. eval) may reallocate Perl stack and therefore pointer + returned by ST(*) macro is invalid. + . + Construction like this: + . + ST(0) = dbd_db_login6_sv(dbh, imp_dbh, dbname, username, password, attribs) ? &PL_sv_yes : &PL_sv_no; + . + where dbd_db_login6_sv() driver function calls eval may lead to + reallocating Perl stack and therefore invalidating ST(0) pointer. + So that construction would cause memory corruption as left part of + assignment is resolved prior executing dbd_db_login6_sv() function. + . + Correct way how to handle this problem: First call dbd_db_login6_sv() + function and then call ST(0) to retrieve stack pointer. + . + In this patch are fixes all occurrences of such constructions. + . + When running perl under valgrind I got memory corruption in DBD::ODBC + driver in that dbd_db_login6_sv() function due to above problem. +Author: Pali <p...@cpan.org> +Origin: upstream, https://github.com/perl5-dbi/dbi/commit/ea99b6aa +Bug: https://security-tracker.debian.org/tracker/CVE-2020-14392 +Forwarded: not-needed +Reviewed-By: Xavier Guimard <y...@debian.org> +Last-Update: 2020-09-10 + +--- a/DBI.xs ++++ b/DBI.xs +@@ -5252,9 +5252,12 @@ + SV * col + SV * ref + SV * attribs ++ PREINIT: ++ SV *ret; + CODE: + DBD_ATTRIBS_CHECK("bind_col", sth, attribs); +- ST(0) = boolSV(dbih_sth_bind_col(sth, col, ref, attribs)); ++ ret = boolSV(dbih_sth_bind_col(sth, col, ref, attribs)); ++ ST(0) = ret; + (void)cv; + + +@@ -5492,21 +5495,27 @@ + FETCH(h, keysv) + SV * h + SV * keysv ++ PREINIT: ++ SV *ret; + CODE: +- ST(0) = dbih_get_attr_k(h, keysv, 0); ++ ret = dbih_get_attr_k(h, keysv, 0); ++ ST(0) = ret; + (void)cv; + + void + DELETE(h, keysv) + SV * h + SV * keysv ++ PREINIT: ++ SV *ret; + CODE: + /* only private_* keys can be deleted, for others DELETE acts like FETCH */ + /* because the DBI internals rely on certain handle attributes existing */ + if (strnEQ(SvPV_nolen(keysv),"private_",8)) +- ST(0) = hv_delete_ent((HV*)SvRV(h), keysv, 0, 0); ++ ret = hv_delete_ent((HV*)SvRV(h), keysv, 0, 0); + else +- ST(0) = dbih_get_attr_k(h, keysv, 0); ++ ret = dbih_get_attr_k(h, keysv, 0); ++ ST(0) = ret; + (void)cv; + + +--- a/Driver.xst ++++ b/Driver.xst +@@ -60,7 +60,7 @@ + #ifdef dbd_discon_all + + # disconnect_all renamed and ALIAS'd to avoid length clash on VMS :-( +-void ++bool + discon_all_(drh) + SV * drh + ALIAS: +@@ -68,7 +68,9 @@ + CODE: + D_imp_drh(drh); + PERL_UNUSED_VAR(ix); +- ST(0) = dbd_discon_all(drh, imp_drh) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_discon_all(drh, imp_drh); ++ OUTPUT: ++ RETVAL + + #endif /* dbd_discon_all */ + +@@ -102,7 +104,7 @@ + MODULE = DBD::~DRIVER~ PACKAGE = DBD::~DRIVER~::db + + +-void ++bool + _login(dbh, dbname, username, password, attribs=Nullsv) + SV * dbh + SV * dbname +@@ -118,14 +120,16 @@ + char *p = (SvOK(password)) ? SvPV(password,lna) : (char*)""; + #endif + #ifdef dbd_db_login6_sv +- ST(0) = dbd_db_login6_sv(dbh, imp_dbh, dbname, username, password, attribs) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_db_login6_sv(dbh, imp_dbh, dbname, username, password, attribs); + #elif defined(dbd_db_login6) +- ST(0) = dbd_db_login6(dbh, imp_dbh, SvPV_nolen(dbname), u, p, attribs) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_db_login6(dbh, imp_dbh, SvPV_nolen(dbname), u, p, attribs); + #else + PERL_UNUSED_ARG(attribs); +- ST(0) = dbd_db_login( dbh, imp_dbh, SvPV_nolen(dbname), u, p) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_db_login( dbh, imp_dbh, SvPV_nolen(dbname), u, p); + #endif + } ++ OUTPUT: ++ RETVAL + + + void +@@ -296,33 +300,38 @@ + CODE: + { + D_imp_dbh(dbh); +- ST(0) = dbd_db_last_insert_id(dbh, imp_dbh, catalog, schema, table, field, attr); ++ SV *ret = dbd_db_last_insert_id(dbh, imp_dbh, catalog, schema, table, field, attr); ++ ST(0) = ret; + } + + #endif + + +-void ++bool + commit(dbh) + SV * dbh + CODE: + D_imp_dbh(dbh); + if (DBIc_has(imp_dbh,DBIcf_AutoCommit) && DBIc_WARN(imp_dbh)) + warn("commit ineffective with AutoCommit enabled"); +- ST(0) = dbd_db_commit(dbh, imp_dbh) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_db_commit(dbh, imp_dbh); ++ OUTPUT: ++ RETVAL + + +-void ++bool + rollback(dbh) + SV * dbh + CODE: + D_imp_dbh(dbh); + if (DBIc_has(imp_dbh,DBIcf_AutoCommit) && DBIc_WARN(imp_dbh)) + warn("rollback ineffective with AutoCommit enabled"); +- ST(0) = dbd_db_rollback(dbh, imp_dbh) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_db_rollback(dbh, imp_dbh); ++ OUTPUT: ++ RETVAL + + +-void ++bool + disconnect(dbh) + SV * dbh + CODE: +@@ -339,8 +348,10 @@ + SvPV(dbh,lna), (int)DBIc_ACTIVE_KIDS(imp_dbh), plural, + "(either destroy statement handles or call finish on them before disconnecting)"); + } +- ST(0) = dbd_db_disconnect(dbh, imp_dbh) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_db_disconnect(dbh, imp_dbh); + DBIc_ACTIVE_off(imp_dbh); /* ensure it's off, regardless */ ++ OUTPUT: ++ RETVAL + + + void +@@ -474,7 +485,7 @@ + MODULE = DBD::~DRIVER~ PACKAGE = DBD::~DRIVER~::st + + +-void ++bool + _prepare(sth, statement, attribs=Nullsv) + SV * sth + SV * statement +@@ -484,11 +495,13 @@ + D_imp_sth(sth); + DBD_ATTRIBS_CHECK("_prepare", sth, attribs); + #ifdef dbd_st_prepare_sv +- ST(0) = dbd_st_prepare_sv(sth, imp_sth, statement, attribs) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_st_prepare_sv(sth, imp_sth, statement, attribs); + #else +- ST(0) = dbd_st_prepare(sth, imp_sth, SvPV_nolen(statement), attribs) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_st_prepare(sth, imp_sth, SvPV_nolen(statement), attribs); + #endif + } ++ OUTPUT: ++ RETVAL + + + #ifdef dbd_st_rows +@@ -505,7 +518,7 @@ + + #ifdef dbd_st_bind_col + +-void ++bool + bind_col(sth, col, ref, attribs=Nullsv) + SV * sth + SV * col +@@ -530,20 +543,21 @@ + } + } + switch(dbd_st_bind_col(sth, imp_sth, col, ref, sql_type, attribs)) { +- case 2: ST(0) = &PL_sv_yes; /* job done completely */ ++ case 2: RETVAL = TRUE; /* job done completely */ + break; + case 1: /* fallback to DBI default */ +- ST(0) = (DBIc_DBISTATE(imp_sth)->bind_col(sth, col, ref, attribs)) +- ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = DBIc_DBISTATE(imp_sth)->bind_col(sth, col, ref, attribs); + break; +- default: ST(0) = &PL_sv_no; /* dbd_st_bind_col has called set_err */ ++ default: RETVAL = FALSE; /* dbd_st_bind_col has called set_err */ + break; + } + } ++ OUTPUT: ++ RETVAL + + #endif /* dbd_st_bind_col */ + +-void ++bool + bind_param(sth, param, value, attribs=Nullsv) + SV * sth + SV * param +@@ -567,12 +581,13 @@ + DBD_ATTRIB_GET_IV(attribs, "TYPE",4, svp, sql_type); + } + } +- ST(0) = dbd_bind_ph(sth, imp_sth, param, value, sql_type, attribs, FALSE, 0) +- ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_bind_ph(sth, imp_sth, param, value, sql_type, attribs, FALSE, 0); + } ++ OUTPUT: ++ RETVAL + + +-void ++bool + bind_param_inout(sth, param, value_ref, maxlen, attribs=Nullsv) + SV * sth + SV * param +@@ -602,9 +617,10 @@ + DBD_ATTRIB_GET_IV(attribs, "TYPE",4, svp, sql_type); + } + } +- ST(0) = dbd_bind_ph(sth, imp_sth, param, value, sql_type, attribs, TRUE, maxlen) +- ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_bind_ph(sth, imp_sth, param, value, sql_type, attribs, TRUE, maxlen); + } ++ OUTPUT: ++ RETVAL + + + void +@@ -640,7 +656,8 @@ + CODE: + { + D_imp_sth(sth); +- ST(0) = dbd_st_execute_for_fetch(sth, imp_sth, fetch_tuple_sub, tuple_status); ++ SV *ret = dbd_st_execute_for_fetch(sth, imp_sth, fetch_tuple_sub, tuple_status); ++ ST(0) = ret; + } + + #endif +@@ -659,7 +676,8 @@ + CODE: + { + D_imp_sth(sth); +- ST(0) = dbd_st_last_insert_id(sth, imp_sth, catalog, schema, table, field, attr); ++ SV *ret = dbd_st_last_insert_id(sth, imp_sth, catalog, schema, table, field, attr); ++ ST(0) = ret; + } + + #endif +@@ -716,7 +734,7 @@ + } + + +-void ++bool + finish(sth) + SV * sth + CODE: +@@ -733,10 +751,12 @@ + XSRETURN_YES; + } + #ifdef dbd_st_finish3 +- ST(0) = dbd_st_finish3(sth, imp_sth, 0) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_st_finish3(sth, imp_sth, 0); + #else +- ST(0) = dbd_st_finish(sth, imp_sth) ? &PL_sv_yes : &PL_sv_no; ++ RETVAL = dbd_st_finish(sth, imp_sth); + #endif ++ OUTPUT: ++ RETVAL + + + void diff --git a/debian/patches/CVE-2020-14393.patch b/debian/patches/CVE-2020-14393.patch new file mode 100644 index 0000000..3429836 --- /dev/null +++ b/debian/patches/CVE-2020-14393.patch @@ -0,0 +1,89 @@ +Description: Fix a buffer overlfow on an overlong DBD class name + dbih_setup_handle() in DBI.xs does: + . + static void + dbih_setup_handle(pTHX_ SV *orv, char *imp_class, SV *parent, SV *imp_datasv) + { + [...] + char imp_mem_name[300]; + [...] + strcpy(imp_mem_name, imp_class); + strcat(imp_mem_name, "_mem"); + [...] + } + . + If imp_class argument string value is longer than 300 - strlen("_mem") + - 1 bytes, a data will be written past imp_mem_name[] array. The + imp_class comes from DBD driver class name (DBI::_new_drh -> + _new_handle() -> dbih_setup_handle()). + . + People usually do not use so long package names (e.g. DBD::ExampleP + calls DBI::_new_drh() in lib/DBD/ExampleP.pm), so the risk is low. + . + Reproducer: + . + $ perl -MDBI -e 'DBI::_new_drh(q{x} x 300, {}, 0)' + *** buffer overflow detected ***: perl terminated + Aborted (core dumped) + . + https://rt.cpan.org/Ticket/Display.html?id=130191 +Author: Petr Pisar <ppi...@redhat.com> +Origin: upstream, https://github.com/perl5-dbi/dbi/commit/36f2a2c5f +Bug: https://rt.cpan.org/Ticket/Display.html?id=130191 +Forwarded: not-needed +Reviewed-By: Xavier Guimard <y...@debian.org> +Last-Update: 2020-09-12 + +--- a/DBI.xs ++++ b/DBI.xs +@@ -1422,7 +1422,7 @@ + SV *dbih_imp_rv; + SV *dbi_imp_data = Nullsv; + SV **svp; +- char imp_mem_name[300]; ++ SV *imp_mem_name; + HV *imp_mem_stash; + imp_xxh_t *imp; + imp_xxh_t *parent_imp; +@@ -1449,10 +1449,9 @@ + if (mg_find(SvRV(h), DBI_MAGIC) != NULL) + croak(errmsg, neatsvpv(orv,0), imp_class, "already a DBI (or ~magic) handle"); + +- strcpy(imp_mem_name, imp_class); +- strcat(imp_mem_name, "_mem"); +- if ( (imp_mem_stash = gv_stashpv(imp_mem_name, FALSE)) == NULL) +- croak(errmsg, neatsvpv(orv,0), imp_mem_name, "unknown _mem package"); ++ imp_mem_name = sv_2mortal(newSVpvf("%s_mem", imp_class)); ++ if ( (imp_mem_stash = gv_stashsv(imp_mem_name, FALSE)) == NULL) ++ croak(errmsg, neatsvpv(orv,0), SvPVbyte_nolen(imp_mem_name), "unknown _mem package"); + + if ((svp = hv_fetch((HV*)SvRV(h), "dbi_imp_data", 12, 0))) { + dbi_imp_data = *svp; +--- a/t/02dbidrv.t ++++ b/t/02dbidrv.t +@@ -4,7 +4,7 @@ $|=1; + + use strict; + +-use Test::More tests => 53; ++use Test::More tests => 54; + + ## ---------------------------------------------------------------------------- + ## 02dbidrv.t - ... +@@ -21,6 +21,16 @@ + use_ok('DBI'); + } + ++## DBI::_new_drh had an internal limit on a driver class name and crashed. ++SKIP: { ++ Test::More::skip "running DBI::PurePerl", 1 if $DBI::PurePerl; ++ eval { ++ DBI::_new_drh('DBD::Test::OverLong' . 'x' x 300, ++ { Name => 'Test', Version => 'Test', }, 42); ++ }; ++ like($@, qr/unknown _mem package/, 'Overlong DBD class name is processed'); ++} ++ + ## ---------------------------------------------------------------------------- + ## create a Test Driver (DBD::Test) + diff --git a/debian/patches/series b/debian/patches/series index 3a41634..a188c1a 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -2,3 +2,5 @@ t__06attrs.t__localefix.patch t__40profile.t__NTP.patch t__80proxy.t___syslogd.patch spelling.patch +CVE-2020-14392.patch +CVE-2020-14393.patch diff --git a/debian/patches/spelling.patch b/debian/patches/spelling.patch index 2ffb778..8874ab9 100644 --- a/debian/patches/spelling.patch +++ b/debian/patches/spelling.patch @@ -7,7 +7,7 @@ Bug: https://rt.cpan.org/Ticket/Display.html?id=114066 --- a/DBI.pm +++ b/DBI.pm -@@ -7884,7 +7884,7 @@ +@@ -7905,7 +7905,7 @@ $self->{_buf} .= shift; # # DBI feeds us pieces at a time, so accumulate a complete line
