Signed arithmetic overflow in function numtostr

This very minor bug report is about a signed overflow in numtostr. When n is LLONG_MIN on a 2's complement machine, -n overflows:

https://github.com/janmojzis/tinyssh/blob/853505fdde353dc3cb8c1fe7d4508dae4e44d8dc/tinyssh/numtostr.c#L24

This is of no practical consequence, except that it triggers warnings in tools used to detect undefined behavior, and these tools are useful to detect the UBs that have more immediate consequences—and are most useful when one doesn't need to investigate reports of currently harmless UB. Also there is no telling what C compilers will be able to do ten years from now in presence of this overflow.

The function has the following structure:

    char *numtostr(char *strbuf, long long n) {
      ...
      unsigned long long nn;
      int flagsign = 0;
      ...
      if (n < 0) {
        n = -n;          // <--- signed overflow
        flagsign = 1;
      }
      nn = n;
      ...                // <--- nn is modified here
      nn = n;

The shortest but perhaps puzzling fix would be to compute - in the unsigned long long type, converting back to long long when storing the result in n:

      if (n < 0) {
        n = -(unsigned long long)n;
        flagsign = 1;
      }

Now there are overflows when converting n to unsigned long long and the result of -(unsigned long long)n back to long long but these are respectively defined and implementation-defined.

A perhaps more readable fix would be to save the absolute value of n, since it is used twice, in an unsigned long long variable.

janmojzis / tinyssh

Signed arithmetic overflow in function numtostr #12