Hi,

Shortly after recallocarray(3) was introduced, it was put into use for
several objects internal to the stdio library "to avoid leaving detritus
in memory when resizing buffers".  However, in the end, this memory is
still released by plain free(3) calls.

The same reason that motivated the change to recallocarray(3) should
also entail that to freezero(3), where applicable.  Currently, a
suitable overflow from a malloc(3)ed buffer could allow an attacker to
retrieve lines previously read by fgetln(3), as well as data previously
written using the fprintf(3) family of functions, even long after the
corresponding streams have been closed and even if the programmer was
very careful explicitly to discard all the manually allocated objects
that could have contained sensitive data.  The diff below makes such
attacks much less likely to succeed, so that one can read and write
secrets to files with stdio functions more safely, id est without the
undesirable side effect of leaving parts of those secrets in heap memory
afterwards.

Index: asprintf.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/asprintf.c,v
retrieving revision 1.25
diff -u -p -r1.25 asprintf.c
--- asprintf.c  17 Mar 2017 14:53:08 -0000      1.25
+++ asprintf.c  26 Oct 2017 04:59:36 -0000
@@ -61,7 +61,7 @@ asprintf(char **str, const char *fmt, ..
        return (ret);
 
 err:
-       free(f._bf._base);
+       freezero(f._bf._base);
        f._bf._base = NULL;
        *str = NULL;
        errno = ENOMEM;
Index: fclose.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/fclose.c,v
retrieving revision 1.10
diff -u -p -r1.10 fclose.c
--- fclose.c    31 Aug 2015 02:53:57 -0000      1.10
+++ fclose.c    26 Oct 2017 04:59:36 -0000
@@ -51,7 +51,7 @@ fclose(FILE *fp)
        if (fp->_close != NULL && (*fp->_close)(fp->_cookie) < 0)
                r = EOF;
        if (fp->_flags & __SMBF)
-               free((char *)fp->_bf._base);
+               freezero(fp->_bf._base);
        if (HASUB(fp))
                FREEUB(fp);
        if (HASLB(fp))
Index: fmemopen.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/fmemopen.c,v
retrieving revision 1.3
diff -u -p -r1.3 fmemopen.c
--- fmemopen.c  31 Aug 2015 02:53:57 -0000      1.3
+++ fmemopen.c  26 Oct 2017 04:59:36 -0000
@@ -107,7 +107,7 @@ fmemopen_close_free(void *v)
 {
        struct state    *st = v;
 
-       free(st->string);
+       freezero(st->string);
        free(st);
 
        return (0);
Index: freopen.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/freopen.c,v
retrieving revision 1.16
diff -u -p -r1.16 freopen.c
--- freopen.c   21 Sep 2016 04:38:56 -0000      1.16
+++ freopen.c   26 Oct 2017 04:59:36 -0000
@@ -106,7 +106,7 @@ freopen(const char *file, const char *mo
        if (isopen && f != wantfd)
                (void) (*fp->_close)(fp->_cookie);
        if (fp->_flags & __SMBF)
-               free((char *)fp->_bf._base);
+               freezero(fp->_bf._base);
        fp->_w = 0;
        fp->_r = 0;
        fp->_p = NULL;
Index: local.h
===================================================================
RCS file: /cvs/src/lib/libc/stdio/local.h,v
retrieving revision 1.25
diff -u -p -r1.25 local.h
--- local.h     23 May 2016 00:21:48 -0000      1.25
+++ local.h     26 Oct 2017 04:59:36 -0000
@@ -82,7 +82,7 @@ __END_HIDDEN_DECLS
 #define        HASUB(fp) (_UB(fp)._base != NULL)
 #define        FREEUB(fp) { \
        if (_UB(fp)._base != (fp)->_ubuf) \
-               free(_UB(fp)._base); \
+               freezero(_UB(fp)._base); \
        _UB(fp)._base = NULL; \
 }
 
@@ -91,7 +91,7 @@ __END_HIDDEN_DECLS
  */
 #define        HASLB(fp) ((fp)->_lb._base != NULL)
 #define        FREELB(fp) { \
-       free((char *)(fp)->_lb._base); \
+       freezero((fp)->_lb._base); \
        (fp)->_lb._base = NULL; \
 }
 
Index: setvbuf.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/setvbuf.c,v
retrieving revision 1.14
diff -u -p -r1.14 setvbuf.c
--- setvbuf.c   21 Sep 2016 04:38:56 -0000      1.14
+++ setvbuf.c   26 Oct 2017 04:59:36 -0000
@@ -70,7 +70,7 @@ setvbuf(FILE *fp, char *buf, int mode, s
        fp->_r = fp->_lbfsize = 0;
        flags = fp->_flags;
        if (flags & __SMBF)
-               free(fp->_bf._base);
+               freezero(fp->_bf._base);
        flags &= ~(__SLBF | __SNBF | __SMBF | __SOPT | __SNPT | __SEOF);
 
        /* If setting unbuffered mode, skip all the hard work. */
Index: vasprintf.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/vasprintf.c,v
retrieving revision 1.22
diff -u -p -r1.22 vasprintf.c
--- vasprintf.c 17 Mar 2017 14:53:08 -0000      1.22
+++ vasprintf.c 26 Oct 2017 04:59:36 -0000
@@ -57,7 +57,7 @@ vasprintf(char **str, const char *fmt, _
        return (ret);
 
 err:
-       free(f._bf._base);
+       freezero(f._bf._base);
        f._bf._base = NULL;
        *str = NULL;
        errno = ENOMEM;
Index: vfprintf.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/vfprintf.c,v
retrieving revision 1.77
diff -u -p -r1.77 vfprintf.c
--- vfprintf.c  29 Aug 2016 12:20:57 -0000      1.77
+++ vfprintf.c  26 Oct 2017 04:59:36 -0000
@@ -199,7 +199,7 @@ __wcsconv(wchar_t *wcsarg, int prec)
        memset(&mbs, 0, sizeof(mbs));
        if ((nbytes = wcsrtombs(convbuf, (const wchar_t **)&p,
            nbytes, &mbs)) == (size_t)-1) {
-               free(convbuf);
+               freezero(convbuf);
                return (NULL);
        }
        convbuf[nbytes] = '\0';
@@ -857,7 +857,7 @@ fp_common:
                        if (flags & LONGINT) {
                                wchar_t *wcp;
 
-                               free(convbuf);
+                               freezero(convbuf);
                                convbuf = NULL;
                                if ((wcp = GETARG(wchar_t *)) == NULL) {
                                        struct syslog_data sdata = 
SYSLOG_DATA_INIT;
@@ -1087,7 +1087,7 @@ overflow:
 
 finish:
 #ifdef PRINTF_WIDE_CHAR
-       free(convbuf);
+       freezero(convbuf);
 #endif
 #ifdef FLOATING_POINT
        if (dtoaresult)
Index: vfwprintf.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/vfwprintf.c,v
retrieving revision 1.18
diff -u -p -r1.18 vfwprintf.c
--- vfwprintf.c 15 Aug 2017 00:20:39 -0000      1.18
+++ vfwprintf.c 26 Oct 2017 04:59:36 -0000
@@ -221,7 +221,7 @@ __mbsconv(char *mbsarg, int prec)
                insize -= nconv;
        }
        if (nconv == (size_t)-1 || nconv == (size_t)-2) {
-               free(convbuf);
+               freezero(convbuf);
                return (NULL);
        }
        *wcp = '\0';
@@ -668,7 +668,7 @@ reswitch:   switch (ch) {
                                prec = dtoaend - dtoaresult;
                        if (expt == INT_MAX)
                                ox[1] = '\0';
-                       free(convbuf);
+                       freezero(convbuf);
                        cp = convbuf = __mbsconv(dtoaresult, -1);
                        if (cp == NULL)
                                goto error;
@@ -717,7 +717,7 @@ fp_begin:
                                if (expt == 9999)
                                        expt = INT_MAX;
                        }
-                       free(convbuf);
+                       freezero(convbuf);
                        cp = convbuf = __mbsconv(dtoaresult, -1);
                        if (cp == NULL)
                                goto error;
@@ -839,7 +839,7 @@ fp_common:
 
                                        mbsarg = "(null)";
                                }
-                               free(convbuf);
+                               freezero(convbuf);
                                convbuf = __mbsconv(mbsarg, prec);
                                if (convbuf == NULL) {
                                        fp->_flags |= __SERR;
@@ -1055,7 +1055,7 @@ overflow:
        ret = -1;
 
 finish:
-       free(convbuf);
+       freezero(convbuf);
 #ifdef FLOATING_POINT
        if (dtoaresult)
                __freedtoa(dtoaresult);
Index: vswprintf.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/vswprintf.c,v
retrieving revision 1.6
diff -u -p -r1.6 vswprintf.c
--- vswprintf.c 31 Aug 2015 02:53:57 -0000      1.6
+++ vswprintf.c 26 Oct 2017 04:59:36 -0000
@@ -64,13 +64,13 @@ vswprintf(wchar_t * __restrict s, size_t
        ret = __vfwprintf(&f, fmt, ap);
        if (ret < 0) {
                sverrno = errno;
-               free(f._bf._base);
+               freezero(f._bf._base);
                errno = sverrno;
                return (-1);
        }
        if (ret == 0) {
                s[0] = L'\0';
-               free(f._bf._base);
+               freezero(f._bf._base);
                return (0);
        }
        *f._p = '\0';
@@ -81,7 +81,7 @@ vswprintf(wchar_t * __restrict s, size_t
         */
        bzero(&mbs, sizeof(mbs));
        nwc = mbsrtowcs(s, (const char **)&mbp, n, &mbs);
-       free(f._bf._base);
+       freezero(f._bf._base);
        if (nwc == (size_t)-1) {
                errno = EILSEQ;
                return (-1);
Index: vswscanf.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/vswscanf.c,v
retrieving revision 1.3
diff -u -p -r1.3 vswscanf.c
--- vswscanf.c  31 Aug 2015 02:53:57 -0000      1.3
+++ vswscanf.c  26 Oct 2017 04:59:36 -0000
@@ -70,7 +70,7 @@ vswscanf(const wchar_t * __restrict str,
        bzero(&mbs, sizeof(mbs));
        strp = str;
        if ((mlen = wcsrtombs(mbstr, &strp, len, &mbs)) == (size_t)-1) {
-               free(mbstr);
+               freezero(mbstr);
                return (EOF);
        }
        if (mlen == len)
@@ -82,7 +82,7 @@ vswscanf(const wchar_t * __restrict str,
        f._read = eofread;
        f._lb._base = NULL;
        r = __vfwscanf(&f, fmt, ap);
-       free(mbstr);
+       freezero(mbstr);
 
        return (r);
 }

Regards,

kshe

Reply via email to