On Sun, 26 Nov 2017 19:56:03 +0000, kshe wrote:
> 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.
The diff previously attached was one of my early drafts where I only
marked the relevant calls. Please ignore it. Here is the real patch I
intended to send.
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 23:30:57 -0000
@@ -61,7 +61,7 @@ asprintf(char **str, const char *fmt, ..
return (ret);
err:
- free(f._bf._base);
+ freezero(f._bf._base, f._bf._size);
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 23:30:57 -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, fp->_bf._size);
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 23:30:57 -0000
@@ -107,7 +107,7 @@ fmemopen_close_free(void *v)
{
struct state *st = v;
- free(st->string);
+ freezero(st->string, st->size);
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 23:30:57 -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->_bf._size);
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 23:30:57 -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)._size); \
_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._size); \
(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 23:30:57 -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, fp->_bf._size);
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 23:30:57 -0000
@@ -57,7 +57,7 @@ vasprintf(char **str, const char *fmt, _
return (ret);
err:
- free(f._bf._base);
+ freezero(f._bf._base, f._bf._size);
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 23:30:57 -0000
@@ -159,7 +159,7 @@ __wcsconv(wchar_t *wcsarg, int prec)
char buf[MB_LEN_MAX];
wchar_t *p;
char *convbuf;
- size_t clen, nbytes;
+ size_t clen, nbytes, end;
/* Allocate space for the maximum number of bytes we could output. */
if (prec < 0) {
@@ -197,12 +197,12 @@ __wcsconv(wchar_t *wcsarg, int prec)
/* Fill the output buffer. */
p = wcsarg;
memset(&mbs, 0, sizeof(mbs));
- if ((nbytes = wcsrtombs(convbuf, (const wchar_t **)&p,
+ if ((end = wcsrtombs(convbuf, (const wchar_t **)&p,
nbytes, &mbs)) == (size_t)-1) {
- free(convbuf);
+ freezero(convbuf, nbytes);
return (NULL);
}
- convbuf[nbytes] = '\0';
+ convbuf[end] = '\0';
return (convbuf);
}
#endif
@@ -857,8 +857,10 @@ fp_common:
if (flags & LONGINT) {
wchar_t *wcp;
- free(convbuf);
- convbuf = NULL;
+ if (convbuf != NULL) {
+ freezero(convbuf, strlen(convbuf));
+ convbuf = NULL;
+ }
if ((wcp = GETARG(wchar_t *)) == NULL) {
struct syslog_data sdata =
SYSLOG_DATA_INIT;
int save_errno = errno;
@@ -1087,7 +1089,8 @@ overflow:
finish:
#ifdef PRINTF_WIDE_CHAR
- free(convbuf);
+ if (convbuf != NULL)
+ freezero(convbuf, strlen(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 23:30:57 -0000
@@ -169,7 +169,7 @@ __mbsconv(char *mbsarg, int prec)
mbstate_t mbs;
wchar_t *convbuf, *wcp;
const char *p;
- size_t insize, nchars, nconv;
+ size_t insize, osize, nchars, nconv;
if (mbsarg == NULL)
return (NULL);
@@ -208,6 +208,7 @@ __mbsconv(char *mbsarg, int prec)
convbuf = calloc(insize + 1, sizeof(*convbuf));
if (convbuf == NULL)
return (NULL);
+ osize = insize;
wcp = convbuf;
p = mbsarg;
bzero(&mbs, sizeof(mbs));
@@ -221,7 +222,7 @@ __mbsconv(char *mbsarg, int prec)
insize -= nconv;
}
if (nconv == (size_t)-1 || nconv == (size_t)-2) {
- free(convbuf);
+ freezero(convbuf, osize * sizeof(*convbuf));
return (NULL);
}
*wcp = '\0';
@@ -668,7 +669,8 @@ reswitch: switch (ch) {
prec = dtoaend - dtoaresult;
if (expt == INT_MAX)
ox[1] = '\0';
- free(convbuf);
+ if (convbuf != NULL)
+ freezero(convbuf, strlen(convbuf));
cp = convbuf = __mbsconv(dtoaresult, -1);
if (cp == NULL)
goto error;
@@ -717,7 +719,8 @@ fp_begin:
if (expt == 9999)
expt = INT_MAX;
}
- free(convbuf);
+ if (convbuf != NULL)
+ freezero(convbuf, strlen(convbuf));
cp = convbuf = __mbsconv(dtoaresult, -1);
if (cp == NULL)
goto error;
@@ -839,7 +842,8 @@ fp_common:
mbsarg = "(null)";
}
- free(convbuf);
+ if (convbuf != NULL)
+ freezero(convbuf, strlen(convbuf));
convbuf = __mbsconv(mbsarg, prec);
if (convbuf == NULL) {
fp->_flags |= __SERR;
@@ -1055,7 +1059,8 @@ overflow:
ret = -1;
finish:
- free(convbuf);
+ if (convbuf != NULL)
+ freezero(convbuf, strlen(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 23:30:57 -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, f._bf._size);
errno = sverrno;
return (-1);
}
if (ret == 0) {
s[0] = L'\0';
- free(f._bf._base);
+ freezero(f._bf._base, f._bf._size);
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, f._bf._size);
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 23:30:57 -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, len);
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, len);
return (r);
}
Regards,
kshe
