File perl-DBI-CVE-2020-14392.patch of Package perl-DBI

Overview Repositories Revisions Requests Users Attributes Meta

File perl-DBI-CVE-2020-14392.patch of Package perl-DBI (Revision 367d86a793805a7718e896498af17ee3)

Currently displaying revision 367d86a793805a7718e896498af17ee3 , Show latest

From ea99b6aafb437db53c28fd40d5eafbe119cd66e1 Mon Sep 17 00:00:00 2001
From: Pali <pali@cpan.org>
Date: Wed, 31 Jul 2019 14:01:35 +0200
Subject: [PATCH] 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.

Exactly same problem was present in Encode module which was fixed in pull
request: https://github.com/dankogai/p5-encode/pull/72
---
 DBI.xs     | 17 ++++++++---
 Driver.xst | 84 +++++++++++++++++++++++++++++++++---------------------
 2 files changed, 65 insertions(+), 36 deletions(-)

Index: DBI-1.628/DBI.xs
===================================================================
--- DBI-1.628.orig/DBI.xs
+++ DBI-1.628/DBI.xs
@@ -5164,9 +5164,12 @@ bind_col(sth, col, ref, attribs=Nullsv)
     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;
 
 
@@ -5404,8 +5407,11 @@ void
 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;
 
 
Index: DBI-1.628/Driver.xst
===================================================================
--- DBI-1.628.orig/Driver.xst
+++ DBI-1.628/Driver.xst
@@ -39,7 +39,7 @@ dbixs_revision(...)
 #ifdef dbd_discon_all
 
 # disconnect_all renamed and ALIAS'd to avoid length clash on VMS :-(
-void
+bool
 discon_all_(drh)
     SV *        drh
     ALIAS:
@@ -47,7 +47,9 @@ discon_all_(drh)
     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 */
 
@@ -81,7 +83,7 @@ data_sources(drh, attr = Nullsv)
 MODULE = DBD::~DRIVER~    PACKAGE = DBD::~DRIVER~::db
 
 
-void
+bool
 _login(dbh, dbname, username, password, attribs=Nullsv)
     SV *        dbh
     SV *        dbname
@@ -97,13 +99,15 @@ _login(dbh, dbname, username, password,
     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
-    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
@@ -266,33 +270,38 @@ last_insert_id(dbh, catalog, schema, tab
     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:
@@ -309,8 +318,10 @@ disconnect(dbh)
             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
@@ -444,7 +455,7 @@ data_sources(dbh, attr = Nullsv)
 MODULE = DBD::~DRIVER~    PACKAGE = DBD::~DRIVER~::st
 
 
-void
+bool
 _prepare(sth, statement, attribs=Nullsv)
     SV *        sth
     SV *        statement
@@ -454,11 +465,13 @@ _prepare(sth, statement, attribs=Nullsv)
     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
@@ -475,7 +488,7 @@ rows(sth)
 
 #ifdef dbd_st_bind_col
 
-void
+bool
 bind_col(sth, col, ref, attribs=Nullsv)
     SV *        sth
     SV *        col
@@ -500,20 +513,21 @@ bind_col(sth, col, ref, attribs=Nullsv)
         }
     }
     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
@@ -537,12 +551,13 @@ bind_param(sth, param, value, attribs=Nu
             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
@@ -572,9 +587,10 @@ bind_param_inout(sth, param, value_ref,
             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
@@ -611,7 +627,8 @@ execute_for_fetch(sth, fetch_tuple_sub,
     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
@@ -669,7 +686,7 @@ fetchall_arrayref(sth, slice=&PL_sv_unde
     }
 
 
-void
+bool
 finish(sth)
     SV *        sth
     CODE:
@@ -686,10 +703,12 @@ finish(sth)
         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

Places

File perl-DBI-CVE-2020-14392.patch of Package perl-DBI (Revision 367d86a793805a7718e896498af17ee3)

Places