Repository : http://git.fedorahosted.org/git/?p=secure-coding.git On branch : master >--------------------------------------------------------------- commit afbe09041885e6a3a92df626851f88c42ef1a243 Merge: 0d2ea66 2067762 Author: Eric Christensen <sparks@xxxxxxxxxxxxxxxxx> Date: Tue Sep 17 20:42:46 2013 -0400 Merge branch 'master' of git+ssh://git.fedorahosted.org/git/secure-coding >--------------------------------------------------------------- defensive-coding/en-US/CXX-Language.xml | 16 +- defensive-coding/en-US/CXX-Std.xml | 161 +++++++++++++++++++- defensive-coding/en-US/Tasks-Serialization.xml | 2 +- .../C-String-Functions-strncat-as-strncpy.xml | 2 +- defensive-coding/src/C-String-Functions.c | 2 +- 5 files changed, 171 insertions(+), 12 deletions(-) diff --git a/defensive-coding/en-US/CXX-Language.xml b/defensive-coding/en-US/CXX-Language.xml index 9dbc4f3..b6e6df9 100644 --- a/defensive-coding/en-US/CXX-Language.xml +++ b/defensive-coding/en-US/CXX-Language.xml @@ -19,8 +19,9 @@ array. Current GCC versions generate code that performs a computation of the form <literal>sizeof(T) * size_t(n) + cookie_size</literal>, where <literal>cookie_size</literal> is - currently at most 8. This computation can overflow, and - GCC-generated code does not detect this. + currently at most 8. This computation can overflow, and GCC + versions prior to 4.8 generated code which did not detect this. + (Fedora 18 was the first release which fixed this in GCC.) </para> <para> The <literal>std::vector</literal> template can be used instead @@ -28,11 +29,12 @@ overflow internally.) </para> <para> - If there is no alternative to <literal>operator new[]</literal>, - code which allocates arrays with a variable length must check - for overflow manually. For the <literal>new T[n]</literal> - example, the size check could be <literal>n || (n > 0 && - n > (size_t(-1) - 8) / sizeof(T))</literal>. (See <xref + If there is no alternative to <literal>operator new[]</literal> + and the sources will be compiled with older GCC versions, code + which allocates arrays with a variable length must check for + overflow manually. For the <literal>new T[n]</literal> example, + the size check could be <literal>n || (n > 0 && n > + (size_t(-1) - 8) / sizeof(T))</literal>. (See <xref linkend="sect-Defensive_Coding-C-Arithmetic"/>.) If there are additional dimensions (which must be constants according to the C++ standard), these should be included as factors in the diff --git a/defensive-coding/en-US/CXX-Std.xml b/defensive-coding/en-US/CXX-Std.xml index 5ed53a4..b221949 100644 --- a/defensive-coding/en-US/CXX-Std.xml +++ b/defensive-coding/en-US/CXX-Std.xml @@ -7,10 +7,142 @@ The C++ standard library includes most of its C counterpart by reference, see <xref linkend="sect-Defensive_Coding-C-Libc"/>. </para> - <section> + <section id="sect-Defensive_Coding-CXX-Std-Functions"> + <title>Functions that are difficult to use</title> + <para> + This section collects functions and function templates which are + part of the standard library and are difficult to use. + </para> + <section id="sect-Defensive_Coding-CXX-Std-Functions-Unpaired_Iterators"> + <title>Unpaired iterators</title> + <para> + Functions which use output operators or iterators which do not + come in pairs (denoting ranges) cannot perform iterator range + checking. + (See <xref linkend="sect-Defensive_Coding-CXX-Std-Iterators"/>) + Function templates which involve output iterators are + particularly dangerous: + </para> + <itemizedlist> + <listitem><para><function>std::copy</function></para></listitem> + <listitem><para><function>std::copy_backward</function></para></listitem> + <listitem><para><function>std::copy_if</function></para></listitem> + <listitem><para><function>std::move</function> (three-argument variant)</para></listitem> + <listitem><para><function>std::move_backward</function></para></listitem> + <listitem><para><function>std::partition_copy_if</function></para></listitem> + <listitem><para><function>std::remove_copy</function></para></listitem> + <listitem><para><function>std::remove_copy_if</function></para></listitem> + <listitem><para><function>std::replace_copy</function></para></listitem> + <listitem><para><function>std::replace_copy_if</function></para></listitem> + <listitem><para><function>std::swap_ranges</function></para></listitem> + <listitem><para><function>std::transform</function></para></listitem> + </itemizedlist> + <para> + In addition, <function>std::copy_n</function>, + <function>std::fill_n</function> and + <function>std::generate_n</function> do not perform iterator + checking, either, but there is an explicit count which has to be + supplied by the caller, as opposed to an implicit length + indicator in the form of a pair of forward iterators. + </para> + <para> + These output-iterator-expecting functions should only be used + with unlimited-range output iterators, such as iterators + obtained with the <function>std::back_inserter</function> + function. + </para> + <para> + Other functions use single input or forward iterators, which can + read beyond the end of the input range if the caller is not careful: + </para> + <itemizedlist> + <listitem><para><function>std::equal</function></para></listitem> + <listitem><para><function>std::is_permutation</function></para></listitem> + <listitem><para><function>std::mismatch</function></para></listitem> + </itemizedlist> + </section> + </section> + <section id="sect-Defensive_Coding-CXX-Std-String"> + <title>String handling with <literal>std::string</literal></title> + <para> + The <literal>std::string</literal> class provides a convenient + way to handle strings. Unlike C strings, + <literal>std::string</literal> objects have an explicit length + (and can contain embedded NUL characters), and storage for its + characters is managed automatically. This section discusses + <literal>std::string</literal>, but these observations also + apply to other instances of the + <literal>std::basic_string</literal> template. + </para> + <para> + The pointer returned by the <function>data()</function> member + function does not necessarily point to a NUL-terminated string. + To obtain a C-compatible string pointer, use + <function>c_str()</function> instead, which adds the NUL + terminator. + </para> + <para> + The pointers returned by the <function>data()</function> and + <function>c_str()</function> functions and iterators are only + valid until certain events happen. It is required that the + exact <literal>std::string</literal> object still exists (even + if it was initially created as a copy of another string object). + Pointers and iterators are also invalidated when non-const + member functions are called, or functions with a non-const + reference parameter. The behavior of the GCC implementation + deviates from that required by the C++ standard if multiple + threads are present. In general, only the first call to a + non-const member function after a structural modification of the + string (such as appending a character) is invalidating, but this + also applies to member function such as the non-const version of + <function>begin()</function>, in violation of the C++ standard. + </para> + <para> + Particular care is necessary when invoking the + <function>c_str()</function> member function on a temporary + object. This is convenient for calling C functions, but the + pointer will turn invalid as soon as the temporary object is + destroyed, which generally happens when the outermost expression + enclosing the expression on which <function>c_str()</function> + is called completes evaluation. Passing the result of + <function>c_str()</function> to a function which does not store + or otherwise leak that pointer is safe, though. + </para> + <para> + Like with <literal>std::vector</literal> and + <literal>std::array</literal>, subscribing with + <literal>operator[]</literal> does not perform bounds checks. + Use the <function>at(size_type)</function> member function + instead. See <xref + linkend="sect-Defensive_Coding-CXX-Std-Subscript"/>. + </para> + <para> + Never write to the pointers returned by + <function>data()</function> or <function>c_str()</function> + after casting away <literal>const</literal>. If you need a + C-style writable string, use a + <literal>std::vector<char></literal> object and its + <function>data()</function> member function. In this case, you + have to explicitly add the terminating NUL character. + </para> + <para> + GCC's implementation of <literal>std::string</literal> is + currently based on reference counting. It is expected that a + future version will remove the reference counting, due to + performance and conformance issues. As a result, code that + implicitly assumes sharing by holding to pointers or iterators + for too long will break, resulting in run-time crashes or worse. + On the other hand, non-const iterator-returning functions will + no longer give other threads an opportunity for invalidating + existing iterators and pointers because iterator invalidation + does not depend on sharing of the internal character array + object anymore. + </para> + </section> + <section id="sect-Defensive_Coding-CXX-Std-Subscript"> <title>Containers and <literal>operator[]</literal></title> <para> - Many containers similar to <literal>std::vector</literal> + Many sequence containers similar to <literal>std::vector</literal> provide both <literal>operator[](size_type)</literal> and a member function <literal>at(size_type)</literal>. This applies to <literal>std::vector</literal> itself, @@ -28,5 +160,30 @@ slightly more verbose. </para> </section> + <section id="sect-Defensive_Coding-CXX-Std-Iterators"> + <title>Iterators</title> + <para> + Iterators do not perform any bounds checking. Therefore, all + functions that work on iterators should accept them in pairs, + denoting a range, and make sure that iterators are not moved + outside that range. For forward iterators and bidirectional + iterators, you need to check for equality before moving the + first or last iterator in the range. For random-access + iterators, you need to compute the difference before adding or + subtracting an offset. It is not possible to perform the + operation and check for an invalid operator afterwards. + </para> + <para> + Output iterators cannot be compared for equality. Therefore, it + is impossible to write code that detects that it has been + supplied an output area that is too small, and their use should + be avoided. + </para> + <para> + These issues make some of the standard library functions + difficult to use correctly, see <xref + linkend="sect-Defensive_Coding-CXX-Std-Functions-Unpaired_Iterators"/>. + </para> + </section> </section> diff --git a/defensive-coding/en-US/Tasks-Serialization.xml b/defensive-coding/en-US/Tasks-Serialization.xml index 456da55..008e75b 100644 --- a/defensive-coding/en-US/Tasks-Serialization.xml +++ b/defensive-coding/en-US/Tasks-Serialization.xml @@ -64,7 +64,7 @@ <itemizedlist> <listitem><para> Python's <package>pickle</package> and <package>cPickle</package> - modules + modules, and wrappers such as <package>shelve</package> </para></listitem> <listitem><para> Perl's <package>Storable</package> package diff --git a/defensive-coding/en-US/snippets/C-String-Functions-strncat-as-strncpy.xml b/defensive-coding/en-US/snippets/C-String-Functions-strncat-as-strncpy.xml index 0895eaf..1f5c8c6 100644 --- a/defensive-coding/en-US/snippets/C-String-Functions-strncat-as-strncpy.xml +++ b/defensive-coding/en-US/snippets/C-String-Functions-strncat-as-strncpy.xml @@ -4,5 +4,5 @@ <!-- Automatically generated file. Do not edit. --> <programlisting language="C"> buf[0] = '\0'; -strncpy(buf, data, sizeof(buf) - 1); +strncat(buf, data, sizeof(buf) - 1); </programlisting> diff --git a/defensive-coding/src/C-String-Functions.c b/defensive-coding/src/C-String-Functions.c index 34d9c31..9f8898a 100644 --- a/defensive-coding/src/C-String-Functions.c +++ b/defensive-coding/src/C-String-Functions.c @@ -70,7 +70,7 @@ main(void) assert(strncmp(buf, data, 9) == 0); //+ C String-Functions-strncat-as-strncpy buf[0] = '\0'; - strncpy(buf, data, sizeof(buf) - 1); + strncat(buf, data, sizeof(buf) - 1); //- } { -- security mailing list security@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/security
