On 10/18/2016 05:15 AM, Bruno Haible wrote:
>
> How about this? You are doing multiple passes through the argument string
> anyway now. (Originally quotearg was meant as a single-pass implementation,
> but it isn't any more.) So
>   - remove the code block of lines 711..720,
>   - instead, before the for(...) loop, determine all_c_and_shell_quote_compat
>     and encountered_single_quote in an extra single pass through the string,
>   - and put the code block from lines 711..720 *BEFORE* the for(...) loop.
>       
> This way, the buffer will only be filled once. => No buffer overrun.
>
> I'm reverting the unintented parts of test-sh-quote.c. You can then decide
> how you want to modify quotearg.c (and sh-quote.c if you want).

In the attached I added a single extra orig_buffersize variable to
control an extra read-only scan.

Pádraig.
>From db3a948221fc4c69dcaac15d0c2b875228b3a4f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbr...@fb.com>
Date: Tue, 18 Oct 2016 13:00:07 -0700
Subject: [PATCH] quotearg: never write beyond the returned length

* lib/quotearg.c (quotearg_buffer_restyled): Switch to a read-only
scan of the string when we initially encounter a single quote when
shell quoting, so that if we then switch to a more concise quoting method
we will not have written beyond that returned length.
This is significant for sh-quote, which has separate routines
to determine the length and do the actual quoting.
* tests/test-quotearg.h: Reinstate the buffer bounds checking
now that we never write more than the returned length.
---
 lib/quotearg.c        | 33 ++++++++++++++++++++++++++++-----
 tests/test-quotearg.h |  5 ++---
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/lib/quotearg.c b/lib/quotearg.c
index 4009073..730b4b3 100644
--- a/lib/quotearg.c
+++ b/lib/quotearg.c
@@ -252,6 +252,7 @@ quotearg_buffer_restyled (char *buffer, size_t buffersize,
 {
   size_t i;
   size_t len = 0;
+  size_t orig_buffersize = 0;
   char const *quote_string = 0;
   size_t quote_string_len = 0;
   bool backslash_escapes = false;
@@ -300,6 +301,8 @@ quotearg_buffer_restyled (char *buffer, size_t buffersize,
       } \
     while (0)
 
+ process_input:
+
   switch (quoting_style)
     {
     case c_maybe_quoting_style:
@@ -544,6 +547,16 @@ quotearg_buffer_restyled (char *buffer, size_t buffersize,
             {
               if (elide_outer_quotes)
                 goto force_outer_quoting_style;
+
+              if (buffersize && ! orig_buffersize)
+                {
+                  /* Just scan string to see if supports a more concise
+                     representation, rather than writing a longer string
+                     but returning the length of the more concise form.  */
+                  orig_buffersize = buffersize;
+                  buffersize = 0;
+                }
+
               STORE ('\'');
               STORE ('\\');
               STORE ('\'');
@@ -713,11 +726,21 @@ quotearg_buffer_restyled (char *buffer, size_t buffersize,
      better to use the apostrophe modifier "\u02BC" if possible, as that
      renders better and works with the word match regex \W+ etc.  */
   if (quoting_style == shell_always_quoting_style && ! elide_outer_quotes
-      && all_c_and_shell_quote_compat && encountered_single_quote)
-    return quotearg_buffer_restyled (buffer, buffersize, arg, argsize,
-                                     c_quoting_style,
-                                     flags, quote_these_too,
-                                     left_quote, right_quote);
+      && encountered_single_quote)
+    {
+      if (all_c_and_shell_quote_compat)
+        return quotearg_buffer_restyled (buffer, orig_buffersize, arg, argsize,
+                                         c_quoting_style,
+                                         flags, quote_these_too,
+                                         left_quote, right_quote);
+      else if (! buffersize && orig_buffersize)
+        {
+          /* Disable read-only scan, and reprocess to write quoted string.  */
+          buffersize = orig_buffersize;
+          len = 0;
+          goto process_input;
+        }
+    }
 
   if (quote_string && !elide_outer_quotes)
     for (; *quote_string; quote_string++)
diff --git a/tests/test-quotearg.h b/tests/test-quotearg.h
index 8fb1fd9..1de7436 100644
--- a/tests/test-quotearg.h
+++ b/tests/test-quotearg.h
@@ -104,10 +104,9 @@ use_quotearg_buffer (const char *str, size_t *len)
   static char buf[100];
   size_t size;
   memset (buf, 0xa5, 100);
-  size = quotearg_buffer (buf, 50, str, *len, NULL);
+  size = quotearg_buffer (buf, 100, str, *len, NULL);
   *len = size;
-  ASSERT ((unsigned char) buf[size] == '\0');
-  ASSERT ((unsigned char) buf[50] == 0xa5);
+  ASSERT ((unsigned char) buf[size + 1] == 0xa5);
   return buf;
 }
 
-- 
2.5.5

Reply via email to