[Secure Coding] master: Merge branch 'master' of git+ssh://git.fedorahosted.org/git/secure-coding (afbe090)

sparks at fedoraproject.org sparks at fedoraproject.org
Wed Sep 18 00:50:31 UTC 2013


Repository : http://git.fedorahosted.org/git/?p=secure-coding.git

On branch  : master

>---------------------------------------------------------------

commit afbe09041885e6a3a92df626851f88c42ef1a243
Merge: 0d2ea66 2067762
Author: Eric Christensen <sparks at fedoraproject.org>
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 &amp;&amp;
-      n &gt; (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 &amp;&amp; n &gt;
+      (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&lt;char&gt;</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);
     //-
   }
   {



More information about the security mailing list