GNU bug report logs - #35256
Bug report for -W argument (maximum width) - minor and not dangerous

Previous Next

Full log

View this message in rfc822 format

From: alec <at> unifiedmathematics.com
To: 35256 <at> debbugs.gnu.org
Subject: bug#35256: Bug report for -W argument (maximum width) - minor and not dangerous
Date: Sat, 13 Apr 2019 12:29:45 +0100

[Message part 1 (text/plain, inline)]

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! 

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.