Dangerous int-to-string conversions

Converting between a string and an int is a problem without a satisfactory solution in C++. Solutions range from itoa, strtol, through std::stringstream to boost::lexical_cast, but every one has its downsides. You have probably already figured out what to do yourself, but have you ever seen this:

// BEWARE !
bool convert(std::string& to, const int& from)
{
  to = from;
  return true;
}

Is this even legal C++? Yes, it compiles fine! Do you know what it does?

You might find it surprising or not that std::basic_string has an assignment from its underlying CharT. So in order to have a one-character string, you can type:

std::string s;
s = 'c';

This is surprising, because you do not have a corresponding (in terms of argument types) constructor. In order to achieve the same effect in a single-phase initialization, you have to use a different constructor:

std::string s(1, 'c');

And now, because int is implicitly convertible to char, our unexpected conversion from int to std::string ‘works’ (under some definition of ‘work’) with the result:

std::string s;
convert(s, 100);
assert(s == "d"); // assuming ASCII encoding

I consider the existence of this assignment a security bug. I was hit by it when I changed the type of one variable in my program from int to std::string and expected that compiler would warn me in all places where I treated my new string as an int. Well, it didn’t because the compiler likes the mixed assignment. Perhaps I shouldn’t have had such an expectation, but what can I say: I did have it. And I want to warn you lest you should fall into the same trap.

About these ads
This entry was posted in programming and tagged , . Bookmark the permalink.

18 Responses to Dangerous int-to-string conversions

    • John says:

      This has most of the downsides of boost::lexical_cast. Specifically it may cause a dynamic memory allocation on some platforms (or if you overload it for a BigInt class, etc., you may have no choice).

      • robdesbois says:

        FYI overloading std::to_string() would yield undefined behaviour as for any declaration added to namespace `std`.

        • One assumes that by “overload” the actual meaning was “add a new `to_string` in the appropriate namespace of the user-defined type that will be found by ADL.”

        • robdesbois says:

          That is a valid solution – I didn’t mean to imply there was no way to make a findable `to_string` for a custom class, but it pays to use the correct term – particularly because there are various differing pros & cons with each approach.

          At a guess though `std::to_string` is not intended as a customisation point for custom types – if it were I’d expect a template function to have been specified instead. Thoughts?

  1. anicolaescu says:

    Hi Andrzej,

    Shouldn’t the compiler emit a warning that the int will be truncated to char? At least mine does (VS2013).

    You have a typo in the first snippet of code, the 4-th line; it was supposed to be to = from.

    • My g++ 4.4 did not. It may be our warning level. Thanks for the type’o alert. It is fixed now.

      • Bas Timmer says:

        Yeah, to get warned about this, you will need to call g++ with -Wconversion. The really crazy thing to me is that -Wconversion is not included in -Wall -Wextra (which is my default, I guess I’ll have to add -Wconversion).

        I like your blog by the way, keep up the good work!

        • szborows says:

          @Bas: here’s explanation from GCC wiki
          “Implicit conversions are very common in C. This tied with the fact that there is no data-flow in front-ends (see next question) results in hard to avoid warnings for perfectly working and valid code. Wconversion is designed for a niche of uses (security audits, porting 32 bit code to 64 bit, etc.) where the programmer is willing to accept and workaround invalid warnings. Therefore, it shouldn’t be enabled if it is not explicitly requested.”

  2. Frank says:

    There slipped a mistake into your first example. You probably meant “to = from;” in line 4.

  3. John says:

    “…every one has its downsides.”
    Agreed. But I think it raises an interesting question: would would an idea C++ mechanism for forming the string representation of an int look like?

    • John says:

      I meant ideal, of course, not idea.

      I would think there should be a function that takes something like a writable string_ref where the result should go.

  4. This exact assignment is what caused issues when I was trying to utilize the operator[] in a json library to implicitly cast to the desired type: http://stackoverflow.com/questions/10743106/ambiguous-stringoperator-call-for-type-with-implicit-conversion-to-int-and-st

  5. garfen says:

    I believe that the expression “assert(s == ‘d’);” within the last example is ill-formed, as there is no matching operator==/conversion available for the argument types. Also, of very minor concern, ‘d’ might not have the value 100 as ‘d’ is the value of the encoded character in the execution-character set (which may be something else than ASCII).

  6. bl says:

    string representation of an int?
    What’s wrong with good ol’ sprintf?
    Nothing faster.
    Safe as milk if you’re careful, or use snprintf for exact buffer length

  7. david stone says:

    -Wconversion on gcc would have caught this bug, but -Wconversion is unusable with typical C and C++ code. For instance, short n = 0; n += 2; triggers a warning due to implicit promotion of the argument to int, and then it warns that int -> short may overflow. The only way to use -Wconversion (and thus catch all int to char implicit conversions) is by consistently using an integer library that is carefully implemented to be safe and still avoid such warnings.

    I have written such an integer library: http://www.doublewise.net/c++/bounded/

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s