From fc1f3908c12eea139747a61e659112ac8290bbe7 Mon Sep 17 00:00:00 2001 From: Olivier Bertrand Date: Sat, 23 Mar 2019 17:51:40 +0100 Subject: [PATCH] - Fix MDEV-15793: Server crash in PlugCloseFile with sql_mode='' Fixed by replacing sprinf by snprintf in ShowValue to avoid buffer overflow. It nows always use a buffer and returns int. modified: storage/connect/tabdos.cpp modified: storage/connect/tabfmt.cpp modified: storage/connect/value.cpp modified: storage/connect/value.h --- storage/connect/tabdos.cpp | 36 +++++++------- storage/connect/tabfmt.cpp | 13 ++--- storage/connect/value.cpp | 97 +++++++++++++++++--------------------- storage/connect/value.h | 16 +++---- 4 files changed, 78 insertions(+), 84 deletions(-) diff --git a/storage/connect/tabdos.cpp b/storage/connect/tabdos.cpp index 29cbbb35765..e424099e812 100644 --- a/storage/connect/tabdos.cpp +++ b/storage/connect/tabdos.cpp @@ -2492,8 +2492,10 @@ bool DOSCOL::SetBuffer(PGLOBAL g, PVAL value, bool ok, bool check) } // endif's Value, Buf_Type // Allocate the buffer used in WriteColumn for numeric columns - if (!Buf && IsTypeNum(Buf_Type)) - Buf = (char*)PlugSubAlloc(g, NULL, MY_MAX(32, Long + Dcm + 1)); + if (!Buf && IsTypeNum(Buf_Type)) + Buf = (char*)PlugSubAlloc(g, NULL, MY_MAX(32, Long + 1)); + else + Buf = (char*)Value->GetTo_Val(); // Because Colblk's have been made from a copy of the original TDB in // case of Update, we must reset them to point to the original one. @@ -2603,8 +2605,8 @@ void DOSCOL::ReadColumn(PGLOBAL g) /***********************************************************************/ void DOSCOL::WriteColumn(PGLOBAL g) { - char *p, *p2, fmt[32]; - int i, k, len, field; + char *p, fmt[32]; + int i, k, n, len, field; PTDBDOS tdbp = (PTDBDOS)To_Tdb; if (trace(2)) @@ -2679,8 +2681,8 @@ void DOSCOL::WriteColumn(PGLOBAL g) case TYPE_DOUBLE: case TYPE_DECIM: strcpy(fmt, (Ldz) ? "%0*.*lf" : "%*.*lf"); - sprintf(Buf, fmt, field + ((Nod && Dcm) ? 1 : 0), - Dcm, Value->GetFloatValue()); + len = field + ((Nod && Dcm) ? 1 : 0); + snprintf(Buf, len, fmt, len, Dcm, Value->GetFloatValue()); len = strlen(Buf); if (Nod && Dcm) @@ -2699,35 +2701,35 @@ void DOSCOL::WriteColumn(PGLOBAL g) throw 31; } // endswitch BufType - p2 = Buf; + n = strlen(Buf); } else // Standard CONNECT format - p2 = Value->ShowValue(Buf, field); + n = Value->ShowValue(Buf, field); if (trace(1)) - htrc("new length(%p)=%d\n", p2, strlen(p2)); + htrc("new length(%p)=%d\n", Buf, n); - if ((len = strlen(p2)) > field) { - sprintf(g->Message, MSG(VALUE_TOO_LONG), p2, Name, field); + if ((len = n) > field) { + sprintf(g->Message, MSG(VALUE_TOO_LONG), Buf, Name, field); throw 31; } else if (Dsp) for (i = 0; i < len; i++) - if (p2[i] == '.') - p2[i] = Dsp; + if (Buf[i] == '.') + Buf[i] = Dsp; if (trace(2)) - htrc("buffer=%s\n", p2); + htrc("buffer=%s\n", Buf); /*******************************************************************/ /* Updating must be done only when not in checking pass. */ /*******************************************************************/ if (Status) { memset(p, ' ', field); - memcpy(p, p2, len); + memcpy(p, Buf, len); if (trace(2)) htrc(" col write: '%.*s'\n", len, p); - } // endif Use + } // endif Status } else // BIN compressed table /*******************************************************************/ @@ -2738,7 +2740,7 @@ void DOSCOL::WriteColumn(PGLOBAL g) sprintf(g->Message, MSG(BIN_F_TOO_LONG), Name, Value->GetSize(), Long); throw 31; - } // endif + } // endif } // end of WriteColumn diff --git a/storage/connect/tabfmt.cpp b/storage/connect/tabfmt.cpp index 63fa2a63668..02720a3089a 100644 --- a/storage/connect/tabfmt.cpp +++ b/storage/connect/tabfmt.cpp @@ -1485,8 +1485,8 @@ void CSVCOL::ReadColumn(PGLOBAL g) /***********************************************************************/ void CSVCOL::WriteColumn(PGLOBAL g) { - char *p, buf[64]; - int flen; + char *p; + int n, flen; PTDBCSV tdbp = (PTDBCSV)To_Tdb; if (trace(2)) @@ -1508,13 +1508,14 @@ void CSVCOL::WriteColumn(PGLOBAL g) /*********************************************************************/ /* Get the string representation of the column value. */ /*********************************************************************/ - p = Value->ShowValue(buf); + p = Value->GetCharString(Buf); + n = strlen(p); if (trace(2)) - htrc("new length(%p)=%d\n", p, strlen(p)); + htrc("new length(%p)=%d\n", p, n); - if ((signed)strlen(p) > flen) { - sprintf(g->Message, MSG(BAD_FLD_LENGTH), Name, p, flen, + if (n > flen) { + sprintf(g->Message, MSG(BAD_FLD_LENGTH), Name, p, n, tdbp->RowNumber(g), tdbp->GetFile(g)); throw 34; } else if (Dsp) diff --git a/storage/connect/value.cpp b/storage/connect/value.cpp index e159efaa989..d9330a68a15 100644 --- a/storage/connect/value.cpp +++ b/storage/connect/value.cpp @@ -1,7 +1,7 @@ /************* Value C++ Functions Source Code File (.CPP) *************/ -/* Name: VALUE.CPP Version 2.8 */ +/* Name: VALUE.CPP Version 2.9 */ /* */ -/* (C) Copyright to the author Olivier BERTRAND 2001-2017 */ +/* (C) Copyright to the author Olivier BERTRAND 2001-2019 */ /* */ /* This file contains the VALUE and derived classes family functions. */ /* These classes contain values of different types. They are used so */ @@ -882,18 +882,16 @@ bool TYPVAL::GetBinValue(void *buf, int buflen, bool go) /* TYPVAL ShowValue: get string representation of a typed value. */ /***********************************************************************/ template -char *TYPVAL::ShowValue(char *buf, int len) +int TYPVAL::ShowValue(char *buf, int len) { - sprintf(buf, Xfmt, len, Tval); - return buf; + return snprintf(buf, len + 1, Xfmt, len, Tval); } // end of ShowValue template <> -char *TYPVAL::ShowValue(char *buf, int len) +int TYPVAL::ShowValue(char *buf, int len) { - // TODO: use snprintf to avoid possible overflow - sprintf(buf, Xfmt, len, Prec, Tval); - return buf; + // TODO: use a more appropriate format to avoid possible truncation + return snprintf(buf, len + 1, Xfmt, len, Prec, Tval); } // end of ShowValue /***********************************************************************/ @@ -1588,10 +1586,17 @@ bool TYPVAL::GetBinValue(void *buf, int buflen, bool go) /***********************************************************************/ /* STRING ShowValue: get string representation of a char value. */ /***********************************************************************/ -char *TYPVAL::ShowValue(char *, int) - { - return Strp; - } // end of ShowValue +int TYPVAL::ShowValue(char *buf, int buflen) +{ + int len = (Null) ? 0 : strlen(Strp); + + if (buf && buf != Strp) { + memset(buf, ' ', buflen + 1); + memcpy(buf, Strp, MY_MIN(len, buflen)); + } // endif buf + + return len; +} // end of ShowValue /***********************************************************************/ /* STRING GetCharString: get string representation of a char value. */ @@ -1800,10 +1805,9 @@ void DECVAL::Reset(void) /***********************************************************************/ /* DECIMAL ShowValue: get string representation right justified. */ /***********************************************************************/ -char *DECVAL::ShowValue(char *buf, int len) +int DECVAL::ShowValue(char *buf, int len) { - sprintf(buf, Xfmt, len, Strp); - return buf; + return snprintf(buf, len + 1, Xfmt, len, Strp); } // end of ShowValue /***********************************************************************/ @@ -1868,14 +1872,13 @@ int DECVAL::CompareValue(PVAL vp) BINVAL::BINVAL(PGLOBAL g, void *p, int cl, int n) : VALUE(TYPE_BIN) { assert(g); -//Len = n; - Len = (g) ? n : (p) ? strlen((char*)p) : 0; + Len = n; Clen = cl; Binp = PlugSubAlloc(g, NULL, Clen + 1); memset(Binp, 0, Clen + 1); if (p) - memcpy(Binp, p, Len); + memcpy(Binp, p, MY_MIN(Len,Clen)); Chrp = NULL; } // end of BINVAL constructor @@ -2264,14 +2267,12 @@ bool BINVAL::GetBinValue(void *buf, int buflen, bool go) /***********************************************************************/ /* BINVAL ShowValue: get string representation of a binary value. */ /***********************************************************************/ -char *BINVAL::ShowValue(char *buf, int len) - { - //int n = MY_MIN(Len, len / 2); - - //sprintf(buf, GetXfmt(), n, Binp); - //return buf; - return (char*)Binp; - } // end of ShowValue +int BINVAL::ShowValue(char *buf, int len) +{ + memset(buf, 0, len + 1); + memcpy(buf, Binp, MY_MIN(len, Len)); + return Len; +} // end of ShowValue /***********************************************************************/ /* BINVAL GetCharString: get string representation of a binary value. */ @@ -2749,43 +2750,33 @@ char *DTVAL::GetCharString(char *p) /***********************************************************************/ /* DTVAL ShowValue: get string representation of a date value. */ /***********************************************************************/ -char *DTVAL::ShowValue(char *buf, int len) - { +int DTVAL::ShowValue(char *buf, int len) +{ + int rv = 0; + if (Pdtp) { - char *p; - if (!Null) { - size_t m, n = 0; + size_t n = 0, m = len + 1; struct tm tm, *ptm = GetGmTime(&tm); - - - - if (Len < len) { - p = buf; - m = len; - } else { - p = Sdate; - m = Len + 1; - } // endif Len if (ptm) - n = strftime(p, m, Pdtp->OutFmt, ptm); + n = strftime(buf, m, Pdtp->OutFmt, ptm); if (!n) { - *p = '\0'; - strncat(p, "Error", m); - } // endif n + *buf = '\0'; + strncat(buf, "Error", m); + rv = 5; + } else + rv = (int)n; - } else { - p = buf; - *p = '\0'; // DEFAULT VALUE ??? - } // endif Null + } else + *buf = '\0'; // DEFAULT VALUE ??? - return p; } else - return TYPVAL::ShowValue(buf, len); + rv = TYPVAL::ShowValue(buf, len); - } // end of ShowValue + return rv; +} // end of ShowValue #if 0 // Not used by CONNECT /***********************************************************************/ diff --git a/storage/connect/value.h b/storage/connect/value.h index 6613e25100a..4f7d9a440fa 100644 --- a/storage/connect/value.h +++ b/storage/connect/value.h @@ -1,7 +1,7 @@ /**************** Value H Declares Source Code File (.H) ***************/ -/* Name: VALUE.H Version 2.3 */ +/* Name: VALUE.H Version 2.4 */ /* */ -/* (C) Copyright to the author Olivier BERTRAND 2001-2017 */ +/* (C) Copyright to the author Olivier BERTRAND 2001-2019 */ /* */ /* This file contains the VALUE and derived classes declares. */ /***********************************************************************/ @@ -117,7 +117,7 @@ class DllExport VALUE : public BLOCK { virtual void SetValue_pvblk(PVBLK blk, int n) = 0; virtual void SetBinValue(void *p) = 0; virtual bool GetBinValue(void *buf, int buflen, bool go) = 0; - virtual char *ShowValue(char *buf, int len = 0) = 0; + virtual int ShowValue(char *buf, int len) = 0; virtual char *GetCharString(char *p) = 0; virtual bool IsEqual(PVAL vp, bool chktype) = 0; virtual bool Compute(PGLOBAL g, PVAL *vp, int np, OPVAL op); @@ -229,7 +229,7 @@ class DllExport TYPVAL : public VALUE { virtual void SetValue_pvblk(PVBLK blk, int n); virtual void SetBinValue(void *p); virtual bool GetBinValue(void *buf, int buflen, bool go); - virtual char *ShowValue(char *buf, int); + virtual int ShowValue(char *buf, int len); virtual char *GetCharString(char *p); virtual bool IsEqual(PVAL vp, bool chktype); virtual bool Compute(PGLOBAL g, PVAL *vp, int np, OPVAL op); @@ -302,7 +302,7 @@ class DllExport TYPVAL: public VALUE { virtual void SetBinValue(void *p); virtual int CompareValue(PVAL vp); virtual bool GetBinValue(void *buf, int buflen, bool go); - virtual char *ShowValue(char *buf, int); + virtual int ShowValue(char *buf, int len); virtual char *GetCharString(char *p); virtual bool IsEqual(PVAL vp, bool chktype); virtual bool Compute(PGLOBAL g, PVAL *vp, int np, OPVAL op); @@ -334,7 +334,7 @@ class DllExport DECVAL: public TYPVAL { // Methods virtual bool GetBinValue(void *buf, int buflen, bool go); - virtual char *ShowValue(char *buf, int); + virtual int ShowValue(char *buf, int len); virtual bool IsEqual(PVAL vp, bool chktype); virtual int CompareValue(PVAL vp); @@ -387,7 +387,7 @@ class DllExport BINVAL: public VALUE { virtual void SetBinValue(void *p); virtual bool GetBinValue(void *buf, int buflen, bool go); virtual int CompareValue(PVAL) {assert(false); return 0;} - virtual char *ShowValue(char *buf, int); + virtual int ShowValue(char *buf, int len); virtual char *GetCharString(char *p); virtual bool IsEqual(PVAL vp, bool chktype); virtual bool FormatValue(PVAL vp, PCSZ fmt); @@ -415,7 +415,7 @@ class DllExport DTVAL : public TYPVAL { virtual void SetValue_psz(PCSZ s); virtual void SetValue_pvblk(PVBLK blk, int n); virtual char *GetCharString(char *p); - virtual char *ShowValue(char *buf, int); + virtual int ShowValue(char *buf, int len); virtual bool FormatValue(PVAL vp, PCSZ fmt); bool SetFormat(PGLOBAL g, PCSZ fmt, int len, int year = 0); bool SetFormat(PGLOBAL g, PVAL valp);