[Boost-bugs] [Boost C++ Libraries] #6193: lexical_cast overflow processing does not always work correctly

Subject: [Boost-bugs] [Boost C++ Libraries] #6193: lexical_cast overflow processing does not always work correctly
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2011-12-01 22:51:42

#6193: lexical_cast overflow processing does not always work correctly
------------------------------------------------------+---------------------
 Reporter:  David W. Birdsall <dave.birdsall@…>       |       Owner:  nasonov     
     Type:  Bugs                                      |      Status:  new         
Milestone:  To Be Determined                          |   Component:  lexical_cast
  Version:  Boost 1.47.0                              |    Severity:  Regression  
 Keywords:  lexical_cast overflow                     |  
------------------------------------------------------+---------------------
 In the attached program, supplying a really long non-zero number with lots
 of trailing zeros is not detected as an invalid number; instead no
 exception is thrown and a value of zero is returned for the parameter.

 Here are some sample executions of the attached program. The first three
 executions behave correctly (note the third throws the expected
 exception); the fourth execution manifests the bug:

 /home/birdsall>./temp.exe --number 16
 The value this code thinks was supplied for --number was: 16
 /home/birdsall>./temp.exe --number 1600000
 The value this code thinks was supplied for --number was: 1600000
 /home/birdsall>./temp.exe --number 160000000000
 ERROR: in option 'number': invalid option value
 Allowed options:
   --help                produce this help message
   --number arg (=0)     a number parameter
 /home/birdsall>./temp.exe --number
 1600000000000000000000000000000000000000
 The value this code thinks was supplied for --number was: 0
 /home/birdsall>

 I found this bug in Boost 1.47.0, and the code snippets below assume that
 version. However, I checked the code in 1.48.0, and in the latest
 repository, and the bug exists there as well.

 The bug does not exist in Boost 1.43.0; the code there functions
 correctly.

 The bug seems to be in lexical_cast.hpp. In the 1.47.0 version, starting
 at line 702, is the following code:

     while ( begin <= end )
        {
        T const new_sub_value = multiplier * 10 * (*end - czero);

        if (*end < czero || *end >= czero + 10
             /* detecting overflow */
             || new_sub_value/10 != multiplier * (*end - czero)
             ||
 static_cast<T>((std::numeric_limits<T>::max)()-new_sub_value) < value
              )
             return false;

         value += new_sub_value;
         multiplier *= 10;
         --end;
         }

 The problem is, there is no check for overflow after the statement,
 "multiplier *= 10". If this statement is executed enough times, and
 overflows enough times, multiplier gets a value of zero. This defeats the
 overflow checking code earlier in the loop; once multiplier becomes zero,
 new_sub_value will be zero no matter what digit *end is. Therefore, value
 remains zero, new_sub_value/10 remains zero and multiplier * anything
 remains zero.

 I fixed the bug in my own copy in the following way. I don't claim that
 this is the most elegant or efficient fix.

     bool multiplier_overflow = false;

     while ( begin <= end )
     {
          T const new_sub_value = multiplier * 10 * (*end - czero);

          if (*end < czero || *end >= czero + 10
              /* detecting overflow */
              || new_sub_value/10 != multiplier * (*end - czero)
              ||
 static_cast<T>((std::numeric_limits<T>::max)()-new_sub_value) < value
              || (multiplier_overflow && (value != 0))
              )
             return false;

          value += new_sub_value;
          T previous_multiplier = multiplier;
          multiplier *= 10;
          // If we let multiplier =* 10 happen enough times (e.g., because
          // of a long string of trailing zeroes), it will overflow
 multiple
          // times and eventually happen to overflow to zero, defeating the
          // overflow checks above. The check below prevents multiplier
          // from becoming zero.
          if (multiplier/10 != previous_multiplier)
          {
               multiplier_overflow = true;  // remember that multiplier
 overflowed
               multiplier = 10;
          }
          --end;
      }

 A similar copy of the bug occurs earlier in the function, at lines 659
 through 674.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/6193>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.