integer wrapping for `last -N`

[vinc17fr]

In src/wtmpdb.c for last -N:

          maxentries = maxentries * 10 + c - '0';

can yield an integer wrapping (maxentries is an unsigned integer, so not really an integer overflow, but unexpected wrapping).

For instance, last -18446744073709551617 outputs only 1 entry.

A fix could probably be something like that:

          if (maxentries > (ULONG_MAX - (c - '0')) / 10)
            maxentries = ULONG_MAX;
          else
            maxentries = maxentries * 10 + c - '0';

There is no such issue with -n because strtoul is used in this case.

[thkukuk]

That's a theoretical issue with all linux last implementations which seems to exist since 34 years. I heavily doubt that any user would type or use such a long history number, it doesn't make any sense.
And even *BSD does not seem to care about the overflow by looking at their code.
So more realistic: your fix would break compatibility with all other last implementations.

[vinc17fr]

I heavily doubt that any user would type or use such a long history number,

Probably. But note that such numbers might occur via scripts.

More importantly, I've also seen that this option is buggy when entering it twice: last -12 -34 behaves like last -1234 instead of probably last -34 (the last value overriding the previous ones). In practice, this breaks the use of default options in shell aliases.

[thkukuk]

Such numbers also don't occur via scripts.

And about using this option multiple times: that's also a problem of all last implementations on Linux since decades.

Use -n and not the legacy shortcut - and everything works as expected.

[vinc17fr]

Such numbers also don't occur via scripts.

Why not?

FYI, people are actually using huge numbers in scripts:

• Find the largest number that Bash arithmetic can handle?
• What is the maximum value of a numeric bash shell variable?

And the fact that Bash uses intmax_t instead of just long probably means that long may not be sufficient in practice (on 32-bit platforms). And a user may want to use the maximum number for anything where a limit should not apply.

And about using this option multiple times: that's also a problem of all last implementations on Linux since decades.

Yes, but this does not imply that it isn't buggy.

Use -n and not the legacy shortcut - and everything works as expected.

The "legacy shortcut" is still documented and not even discouraged.

[andy-bower]

Hi @vinc17fr,

I contributed this feature because -N is massively ingrained usage for me and I thought it was worth it and I wanted to upgrading users to have the best experience - thanks to @thkukuk for acepting it! Sorry it's not perfect.

I had already considered two alternative implementations (pre-processing the option list before getopt(3) or having a flag set with every non-[0-9] option which will help us process the numerical options more intelligently, but the complexity always seemed over the top and the fact this method mirrors the behaviour of the older tools clinched it for me.

I'm not convinced the huge number issue is going to affect people, not because people don't use huge numbers with shells, but because this feature is essentially a convenience for interactive users.

Knowing how it works I would only consider the -nN approach in a script but I accept that might not be obvious to all. I wonder if explaining it away in the man page would be a bit excessive, though. Another option might be simply to remove the mention from the man page. I would be sad if the feature went though!

[vinc17fr]

Hi @andy-bower,

Perhaps just add to the man page something like "The -N form must not be used if a limit has been set with a previous option." or "The -N form cannot be used to override a previously set limit." (And this should be sufficient to discourage its use in scripts, thus also avoiding the issue with huge numbers.)

[thkukuk]

I'm not convinced the huge number issue is going to affect people, not because people don't use huge numbers with shells, but because this feature is essentially a convenience for interactive users.

I couldn't find a last implementation which has not the same problem with large numbers as reported here, so this is clearly nothing from practical relevance. Else somebody would have fixed it in the last 34 years or so.