[committed] libstdc++: Replace operator>>(istream&, char*) [LWG 2499]

[Jonathan Wakely]

On 05/08/20 22:25 +0100, Jonathan Wakely wrote:
>P0487R1 resolved LWG 2499 for C++20 by removing the operator>> overloads
>that have high risk of buffer overflows. They were replaced by
>equivalents that only accept a reference to an array, and so can
>guarantee not to write past the end of the array.
>
>In order to support both the old and new functionality, this patch
>introduces a new overloaded __istream_extract function which takes a
>maximum length. The new operator>> overloads use the array size as the
>maximum length. The old overloads now use __builtin_object_size to
>determine the available buffer size if available (which requires -O2) or
>use numeric_limits<streamsize>::max()/sizeof(char_type) otherwise. This
>is a change in behaviour, as the old overloads previously always used
>numeric_limits<streamsize>::max(), without considering sizeof(char_type)
>and without attempting to prevent overflows.
>
>Because they now do little more than call __istream_extract, the old
>operator>> overloads are very small inline functions. This means there
>is no advantage to explicitly instantiating them in the library (in fact
>that would prevent the __builtin_object_size checks from ever working).
>As a result, the explicit instantiation declarations can be removed from
>the header. The explicit instantiation definitions are still needed, for
>backwards compatibility with existing code that expects to link to the
>definitions in the library.
>
>While working on this change I noticed that src/c++11/istream-inst.cc
>has the following explicit instantiation definition:
>  template istream& operator>>(istream&, char*);
>This had no effect (and so should not have been present in that file),
>because there was an explicit specialization declared in <istream> and
>defined in src/++98/istream.cc. However, this change removes the
>explicit specialization, and now the explicit instantiation definition
>is necessary to ensure the symbol gets defined in the library.

>Martin, Jakub, could you please double-check the usage of
>__builtin_object_size? (line 805 in libstdc++-v3/include/std/istream)
>Do you see any problems with using it here? If it can't tell the size
>then we just assume it's larger than the string to be extracted, which
>is what the old code did anyway. I do have an idea for stronger
>checking in debug mode, which I'll post in a minute.


The attached (uncommitted and not fully tested) patch would make the
new __istream_extract functions return a bool indicating whether
extraction stopped because the maximum number of characters was
reached (as opposed to reaching EOF or whitespace).

This would allow us to abort the program with a debug mode assertion
if limiting the size to the __builtin_object_size value prevented a
buffer overflow from occurring.



-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 4810 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20200805/fdff7ea7/attachment.bin>