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) + + -- 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/series b/debian/patches/series index 3a41634..969068f 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -2,3 +2,4 @@ t__06attrs.t__localefix.patch t__40profile.t__NTP.patch t__80proxy.t___syslogd.patch spelling.patch +CVE-2020-14392.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