-
Notifications
You must be signed in to change notification settings - Fork 338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
POSIX deviation: test arithmetic operands must support decimal only #498
Comments
Still better than GNU behavior...
08 isn't valid octal. So what exactly is GNU test being accurate to?
Toybox aliases [ to [[, so they are both functionally the same thing. Another cool thing you can do by parsing numbers with atolx:
Not portable to what? GNU and busybox have it, FreeBSD also has it and therefore MacOS does too, but admittedly OpenBSD doesn't looking at their strftime manpage. Which means we are not portable to a OpenBSD environment where toybox test is being used, but toybox date isn't. A quick hack to fix this is to swap out atolx() with regular atol() (Line 82 toys/posix/test.c is where the magic happens). But to keep true POSIX and Bash compatibility there would need to be some way to detect if we are calling under "[[", and only do the octal numbers and such if so. The patch is on the list, but I have my doubts on Rob applying it due to the loss of functionality. If you ever are working on a OpenBSD box and trying to interoperate with zero padded month numbers that you can't turn off with OpenBSD strftime, but can use in toybox test. And it's really super security critical (How exactly is a system supposed to be attacked with this minor bug? Then again I could say that about most of the modern CVE's bugs), but also speed critical enough to not pipe the numbers through |
That's the whole point of this issue. 08 and 09 are also decimal numbers and must be interpreted as such, not octal. That's not just GNU test. That's virtually all test implementations in existence including the
Note that
As I mentioned in my initial report, it's fine to keep that as that's outside POSIX scope (behaviour unspecified).
My view is rather that a portable script will use |
We can do both since one is a complete (Apparently, mostly complete) subset of the other.
The problem is that there is no easy way from my knowledge to keep one and not the other, I may be wrong though.
Like I said, you can pipe numbers through |
FWIW,
They have incompatible syntax and feature set in all shells that have both. [...] It would be enough to do:
I would think (the
You need something like:
|
Re: your patch Note that there's no security issue in toybox' |
"if (operand-has-only-digits[-and-whitespace])" is a oversimplification of what that code would This is something that can be done, but it adds complexity, adds executable size, I may be wrong, and the support for prefixes and hex numbers in test is actually Maybe there should be some way to turn off octal support for atolx. I just found atolx is used in commands like head and tail too, I originally thought it was how lib/args.c parsed Octal is obsolete almost everywhere except file permissions. I'd suggest removing it from atolx entirely, but doing that has the same problems of detecting a octal number and parsing it as decimal, but not a hexadecimal one And of course, similar discussion about this problem for dd is on the list:
On Posix Being the be-all, end-all of implementation details:
Also:
(This is something I was considering suggesting):
And Finally:
|
After more consideration, removing octal support in atolx seems like a better solution. |
Seems sensible to me. Those 0123 treated as octal have been a plague since forever. The Those 0123 treated as octal have already been removed from arithmetic expressions of some shells including mksh and ksh93 (and defunct ksh2020) when not in posix mode (ksh being the shell that introduced them in the first place). zsh never treated those as octal (though added a octalzeroes option in 3.1.7 in 2000 for its sh emulation mode). |
I'm always a little confused about the "do less" reports. I have a pending one in sed on my todo list too. I can see leading zeroes indicating octal being considered confusing to a modern audience, now that 6 bit systems are obsolete and the only fossil use for it that comes to mind is unix permissions. That said, strtol() detects that when it detects 0x and that's libc plumbing, I didn't write that, just used it. Haven't decided what to do about it yet, the reporter of the sed one hadn't actually hit a case where it caused a problem, they were just checking compliance corner cases. Did you have a use case here, or were you just doing compliance checking too? |
Yes,
I'm not a (direct) user of Having said that, questions about why In the general case, in Korn-like shells |
GNU printf doesn't do octal, although busybox does:
|
Note that there are two GNU implementations of the standard I find that both do octal here on Debian with GNU coreutils 9.4 and bash 5.2. According to Most shells have the |
returns false and
gives an error.
POSIX does require operands to be treated as decimal which means it's an undocumented deviation.
The fact that
[
arithmetic operators accept only decimal in bash/zsh while[[ ... ]]
accepts any arithmetic expression (including octal constants if not disabled globally), is a reason why[
is often recommended over[[
or((
in those shells (in particular[[ $attacker_controlled_variable -lt 10 ]]
and(( attacker_controlled_variable < 10 ))
are command injection vulnerabilities there).It's useful to be able to rely on decimal handling when handling 0-padded number such as date/time components as day, month, hour, minute, second are usually 0-padded as in:
(toybox date supports
date +%-m
but that's not standard/portable)If you decide to keep the POSIX deviation, I would suggest that at least sequences of digits that contain 8 and 9 digits be treated as decimal (both 010 and 08 to be considered as number 8) so we can still do date manipulation without having to strip the leading 0s manually.
POSIX doesn't specify the behaviour for
test 0xf -eq 15
ortest 1c -eq 1
ortest 1000 -eq 1kd
so it's fine in that regard to return true instead of an error.The text was updated successfully, but these errors were encountered: