Bug 65142 (CVE-2015-5276) - std::random_device Ignores Read Return Code (CVE-2015-5276)
Summary: std::random_device Ignores Read Return Code (CVE-2015-5276)
Status: RESOLVED FIXED
Alias: CVE-2015-5276
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: 4.9.4
Assignee: Jonathan Wakely
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-20 16:05 UTC by Lee Clagett
Modified: 2015-10-02 20:53 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-02-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lee Clagett 2015-02-20 16:05:56 UTC
When std::random_device reads from the designated random file, it does not check the return code from the read. libc++ throws an exception if 0 or less bytes were read. Without this check, the returned value is completely or partially unitialized.
Comment 1 Jonathan Wakely 2015-02-20 16:53:07 UTC
We should do the same, [rand.device] says:

Throws: A value of an implementation-defined type derived from exception if a random number could not be obtained.
Comment 2 Jonathan Wakely 2015-09-11 13:44:16 UTC
Fixed on trunk so far.
Comment 3 Jonathan Wakely 2015-09-11 13:44:57 UTC
Author: redi
Date: Fri Sep 11 13:44:26 2015
New Revision: 227687

URL: https://gcc.gnu.org/viewcvs?rev=227687&root=gcc&view=rev
Log:
Check read() result in std::random_device.

	PR libstdc++/65142
	* src/c++11/random.cc (random_device::_M_getval()): Check read result.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/src/c++11/random.cc
Comment 4 Florian Weimer 2015-09-14 13:29:09 UTC
This has been assigned CVE-2015-5276.
Comment 5 Florian Weimer 2015-09-14 14:47:47 UTC
The fix is incomplete because short reads can happen in practice for /dev/random at least.

The usual retry loop is needed.  It is not clear what to do on EINTR.
Comment 6 Jonathan Wakely 2015-09-14 15:03:11 UTC
And the check is wrong in the fread() case as it will only ever return 0 or 1.
Comment 7 Richard Biener 2015-09-15 10:39:19 UTC
If the user controls how the random file is opened (non-blocking or blocking)
then the behavior (whether to re-try on EINTR or short reads) should be
controlled by that choice.  Starting to throw on users that don't expect that
would be bad.
Comment 8 Jonathan Wakely 2015-09-15 11:43:56 UTC
(In reply to Richard Biener from comment #7)
> If the user controls how the random file is opened (non-blocking or blocking)

They don't.

> then the behavior (whether to re-try on EINTR or short reads) should be
> controlled by that choice.  Starting to throw on users that don't expect that
> would be bad.

The function is specified to throw on error by the standard.
Comment 9 Jonathan Wakely 2015-09-15 11:49:58 UTC
(In reply to Jonathan Wakely from comment #8)
> (In reply to Richard Biener from comment #7)
> > If the user controls how the random file is opened (non-blocking or blocking)
> 
> They don't.

To be clear, it's always opened blocking with std::fopen(fname, "rb")

New proposed patch at https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01050.html
Comment 10 rguenther@suse.de 2015-09-15 12:12:39 UTC
On Tue, 15 Sep 2015, redi at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65142
> 
> --- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #7)
> > If the user controls how the random file is opened (non-blocking or blocking)
> 
> They don't.
> 
> > then the behavior (whether to re-try on EINTR or short reads) should be
> > controlled by that choice.  Starting to throw on users that don't expect that
> > would be bad.
> 
> The function is specified to throw on error by the standard.

Well, the question is what is an "error" then.  The need to wait
(as you say we open blocking) isn't in my view.  Getting EINTRed
while waiting neither.  Getting a fatal error from the read yes.
Comment 11 Jonathan Wakely 2015-09-17 15:07:17 UTC
Author: redi
Date: Thu Sep 17 15:06:42 2015
New Revision: 227872

URL: https://gcc.gnu.org/viewcvs?rev=227872&root=gcc&view=rev
Log:
Make std::random_device retry after short reads

	PR libstdc++/65142
	* src/c++11/random.cc (random_device::_M_getval()): Retry after short
	reads.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/src/c++11/random.cc
Comment 12 Richard Biener 2015-10-02 13:00:00 UTC
Can you please backport if the solution is the final one?
Comment 13 Jonathan Wakely 2015-10-02 20:08:36 UTC
Author: redi
Date: Fri Oct  2 20:08:04 2015
New Revision: 228419

URL: https://gcc.gnu.org/viewcvs?rev=228419&root=gcc&view=rev
Log:
Backport PR libstdc++/65142 fix from mainline

	PR libstdc++/65142
	* src/c++11/random.cc (random_device::_M_getval()): Check read result
	and retry after short reads.

Modified:
    branches/gcc-5-branch/libstdc++-v3/ChangeLog
    branches/gcc-5-branch/libstdc++-v3/src/c++11/random.cc
Comment 14 Jonathan Wakely 2015-10-02 20:11:45 UTC
Do we want this on gcc-4_9-branch too?
Comment 15 Florian Weimer 2015-10-02 20:15:25 UTC
(In reply to Jonathan Wakely from comment #14)
> Do we want this on gcc-4_9-branch too?

Yes, I think so.  It's non-invasive, and it's not in templates/inline functions, so those who use dynamic linking would actually benefit from it.
Comment 16 Jonathan Wakely 2015-10-02 20:52:06 UTC
Author: redi
Date: Fri Oct  2 20:51:34 2015
New Revision: 228424

URL: https://gcc.gnu.org/viewcvs?rev=228424&root=gcc&view=rev
Log:
Backport PR libstdc++/65142 fix from mainline

	PR libstdc++/65142
	* src/c++11/random.cc (random_device::_M_getval()): Check read result
	and retry after short reads.

Modified:
    branches/gcc-4_9-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_9-branch/libstdc++-v3/src/c++11/random.cc
Comment 17 Jonathan Wakely 2015-10-02 20:53:09 UTC
Fixed for 4.9.4 and 5.3 then.