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.
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.
Fixed on trunk so far.
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
This has been assigned CVE-2015-5276.
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.
And the check is wrong in the fread() case as it will only ever return 0 or 1.
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.
(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.
(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
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.
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
Can you please backport if the solution is the final one?
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
Do we want this on gcc-4_9-branch too?
(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.
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
Fixed for 4.9.4 and 5.3 then.