Non-stub setvbuf

Non-stub setvbuf

[Rich Felker]

Right now, musl's stdio setvbuf function does nothing but set the
buffering mode; it does not honor the buffer provided by the caller.
This is perfectly conforming (whether or how the buffer is used is
unspecified), but I realized from the recent thread about OpenSSH's
CVE-2016-0777 on oss-security that a non-stub setvbuf admits a nice
type of hardening:

http://www.openwall.com/lists/oss-security/2016/01/15/15

In short, the application has no way to scrub implementation-internal
stdio buffers that might contain sensitive data read from or written
to files, but it can scrub buffers it provides via setvbuf. So, I'd
like to start actually using the latter, so that apps that attempt
this hardening measure can benefit from it on musl like they would on
other implementations.

The logic I have in mind is something like:

- Ignore application-provided buffers smaller than UNGET (8) bytes,
  possibly plus some reasonable epsilon, and either turn off buffering
  or keep the internal buffer in this case.

- If the application-provided buffer is larger than something
  threshold M (maybe 16k) and the file mode admits reading, only use
  the first M bytes of the buffer. Otherwise repeated fseek+getc is
  very slow (uselessly refilling a large buffer each time).
  Alternatively the logic to limit read buffer size could go in
  __stdio_read.

Does this sound reasonable?

I also reviewed the lack of locking in setvbuf, which looks like it
risks data races accessing f->flags, but it actually seems fine since
the only code that could run concurrently with it without invoking UB
is __stdio_exit (or perhaps fflush(NULL)) which just checks f->rpos,
f->wpos, etc. Perhaps some comments should be added to this effect,
though.

Rich

Re: Non-stub setvbuf

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

Am Samstag, den 16.01.2016, 20:43 -0500 schrieb Rich Felker:
> Right now, musl's stdio setvbuf function does nothing but set the
> buffering mode; it does not honor the buffer provided by the caller.
> This is perfectly conforming (whether or how the buffer is used is
> unspecified), but I realized from the recent thread about OpenSSH's
> CVE-2016-0777 on oss-security that a non-stub setvbuf admits a nice
> type of hardening:
> 
> http://www.openwall.com/lists/oss-security/2016/01/15/15
> 
> In short, the application has no way to scrub implementation-internal
> stdio buffers that might contain sensitive data read from or written
> to files, but it can scrub buffers it provides via setvbuf. So, I'd
> like to start actually using the latter, so that apps that attempt
> this hardening measure can benefit from it on musl like they would on
> other implementations.

How about just using setvbuf as an indication that the user wants the
buffer to be scrubbed? And so just zero it?

I wouldn't expect setvbuf to be used in places that are performance
critical, so an additional memset shouldn't do much harm, I think.

Jens


-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: Non-stub setvbuf

[Markus Wichmann]

On Sun, Jan 17, 2016 at 09:59:52AM +0100, Jens Gustedt wrote:
> Am Samstag, den 16.01.2016, 20:43 -0500 schrieb Rich Felker:
> > Right now, musl's stdio setvbuf function does nothing but set the
> > buffering mode; it does not honor the buffer provided by the caller.
> > This is perfectly conforming (whether or how the buffer is used is
> > unspecified), but I realized from the recent thread about OpenSSH's
> > CVE-2016-0777 on oss-security that a non-stub setvbuf admits a nice
> > type of hardening:
> > 
> > http://www.openwall.com/lists/oss-security/2016/01/15/15
> > 
> > In short, the application has no way to scrub implementation-internal
> > stdio buffers that might contain sensitive data read from or written
> > to files, but it can scrub buffers it provides via setvbuf. So, I'd
> > like to start actually using the latter, so that apps that attempt
> > this hardening measure can benefit from it on musl like they would on
> > other implementations.
> 
> How about just using setvbuf as an indication that the user wants the
> buffer to be scrubbed? And so just zero it?
> 
> I wouldn't expect setvbuf to be used in places that are performance
> critical, so an additional memset shouldn't do much harm, I think.
> 

That isn't the use everyone has for it. I routinely set stdout and stdin
to line buffered in my programs, because glibc sets the buffering mode
for these streams based on file type, and I want to be consistent in all
cases. And those applications can be performance critical.

But then, I provide no buffer to these calls, so maybe differentiate on
that?

OTOH, full control over buffering is only possible on the syscall level
anyway, so maybe FILEs are inherently insecure for these purposes?

Ciao,
Markus

Re: Non-stub setvbuf

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

Am Sonntag, den 17.01.2016, 12:03 +0100 schrieb Markus Wichmann:
> On Sun, Jan 17, 2016 at 09:59:52AM +0100, Jens Gustedt wrote:
> > How about just using setvbuf as an indication that the user wants the
> > buffer to be scrubbed? And so just zero it?
> > 
> > I wouldn't expect setvbuf to be used in places that are performance
> > critical, so an additional memset shouldn't do much harm, I think.
> > 
> 
> That isn't the use everyone has for it. I routinely set stdout and stdin
> to line buffered in my programs, because glibc sets the buffering mode
> for these streams based on file type, and I want to be consistent in all
> cases. And those applications can be performance critical.
> 
> But then, I provide no buffer to these calls, so maybe differentiate on
> that?

Yes, sorry, I wasn't clear enough. This is what I meant, only scrub
buffers if the user provides a buffer argument.

Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]