Subject: [PATCH] size option may want to reject "bB" or "biB" suffix

From: Young Mo Kang <[email-masked]>
Date: Thu, 28 Apr 2016 01:29:21 +0900

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-masked]>
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