Hello, I found a bug in coreutils' size option, which currently accepts options like "split -b 1bB" or "split -b 1biB". I believe these options should be rejected. I looked through the code and found out that gnulib's __xstrtol function in xstrtol.c is the culprit. I did a quick fix and the patch is attached. The patch should fix this issue in general.
Additionally, while looking at the code, I may have found another bug, but I am not so sure whether this is how it is intended. When I run "shred -s 1B", I think it should shred only a single byte, but it seems to shred 1024 bytes instead. Is this behavior intended? Anyways, I have marked this down in the patch as a FIXME comment. Since the patch applies to gnulib, I am not sure whether this patch should be submitted to gnulib bug report instead. Please let me know if so. Lastly, I noticed that different programs within coreutils accept different size suffixes. For example, split's valid suffix is "bEGKkMmPTYZ0" while shred's is "cbBkKMGTPEZY0". I thought maybe it is better to unify valid suffix for all the programs within coreutils. Best, Young Mo
From 9290639bb9fd8bc999d84b8422a344c3eb990f2c Mon Sep 17 00:00:00 2001 From: Young Mo Kang <[email protected]> Date: Thu, 28 Apr 2016 00:41:12 +0900 Subject: [PATCH] Reject unaccepted suffixes, such as 'bB' or 'biB' * xstrtol.c (__xstrtol): When optional second suffix is allowed, check the first suffix for 'c', 'b', and 'w'. Produce an error when these are followed by the second suffix. --- lib/xstrtol.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/xstrtol.c b/lib/xstrtol.c index 5a3afb5..b117382 100644 --- a/lib/xstrtol.c +++ b/lib/xstrtol.c @@ -157,6 +157,20 @@ __xstrtol (const char *s, char **ptr, int strtol_base, a power of 1024. If no suffix (e.g. "100M"), assume power-of-1024. */ + if (p[0][1] != '\0') + { + switch (**p) + { + case 'b': + case 'c': + case 'w': /* When the second suffix is allowed, + 'b', 'c', or 'w' shall not be the first suffix. + For example, 'biB', 'bB', or 'bD' shall return error */ + *val = tmp; + return err | LONGINT_INVALID_SUFFIX_CHAR; + } + } + switch (p[0][1]) { case 'i': @@ -179,6 +193,8 @@ __xstrtol (const char *s, char **ptr, int strtol_base, break; case 'B': + /* FIXME: Is this correct? Doesn't 'B' represent a single byte, + * which should be equivalent to 'c'? */ overflow = bkm_scale (&tmp, 1024); break; -- 2.7.4
