[bug-diffutils] bug#35256: Bug report for -W argument (maximum width) - minor and not dangerous

[alec]

Hello there.

I was hoping to view a side-by-side diff of something and, perhaps unfairly, was hoping for a setting where diff would choose a width such that there were no truncations and I would use less with no wrapping to inspect the results.

My first attempt was "-W 0" (a width of 0 has no "legit" meaning afterall) - error, so I tried -1. This leads to a weird situation where it seems to just output loads of tabs - while it'll probably still terminate eventually the behaviour is unreasonable.

To try this yourself run something like:

diff -y ./maps ./task/4974/maps -W -1

from /proc/XXXX where XXXX is some PID for a program with threads (eg firefox) and the 4974 is any task that isn't XXXX

ADDENDUM: The -1 isn't important, 9999999999999 also illustrates the problem - END ADDENDUM

Looking at the code (in the 3.7 tarball, src/diff.c modified on 18th of December 2018) notice:

Line 284:
  uintmax_t numval;
Line 525:
    case 'W':
      numval = strtoumax (optarg, &numend, 10);
      if (! (0 < numval && numval <= SIZE_MAX) || *numend)
        try_help ("invalid width '%s'", optarg);
      if (width != numval)
        {
          if (width)
        fatal ("conflicting width options");
          width = numval;
        }
      break;

For convenience:
      uintmax_t strtoumax(const char *nptr, char **endptr, int base);
and it may set errno, my man page doesn't say whether this -1 behaviour is "okay" however it probably is, unsigned afterall, this means that numval is going to be a really really big value.


ABUSE POTENTIAL:

Just basic DOS (denial-of-service) stuff, a CPU usage spike comes from diff itself and it seems to output a lot of tabs (a good 275mib / sec on my machine) and will probably do so for a good few years before anything else comes out, a testament to the robustness of diff is that it did this, and its memory usage didn't start ballooning.

I know diff is used by A LOT of other programs, some of which are web-accessible (eg mediawiki uses diff - and will by default if it finds it), many of my projects use it too. It is not a big stretch to imagine someone has a web-service out there which allows side-by-side format, and not much of a further leap to assume that someone might have an input box for width which exposes -W, guarded only by a regex of the form ^[1-9][0-9]* (which yes, wont allow -1 but will allow 9999999999999)

You could bring a server to its knees pretty quickly using just diff's CPU usage and a few tabs using this - that's not even considering whether or not the system hypothesised here doesn't have trouble with memory from a convenient get_line() function first.

While not really diff's fault or problem, a potential solution detailed below would fix it and not cause any problems for those with legit (?) needs for really wide diffs



SUGGESTIONS:
Humans are limiting here, improvements and the growth of computers wont really affect the maximum width so putting a limit in place is reasonable. I make no claim there is a "maximum useful width" so being able to override will ensure my half-assed musings on such a limit wont cause any problems in the future.

I'd go with something like
#define REASONABLE_LIMIT 1000

Add a check that numval is <= get_reasonable_specified_width_limit() after the existing checks, if not output an error in the form of:

"You probably don't want to do that, see [wherever], if you do specify --we-have-evolved-cylindical-lenses-now or set the environmental variable GNU_DIFF_REASONABLE_LIMIT to a new limit, using 0 for none"

Lastly, for what it's worth from a perfect stranger:

I'm very impressed that diff didn't start consuming huge amounts of memory, and a little saddened that it is impressive!

Thanks very much for diff and your work on it, you have no idea how many things it underpins!