Skip to contentSkip to conversation

Fix commit e473b7c if an inline file happens to have a line break exactly at buffer limit

PATCH In review Arne Schwabe 3 participants 3 messages Jul 27, 2015

lore
Share & tools
lore

Arne Schwabe

author
#0 lore raw

PATCH 1/1 Fix commit e473b7c if an inline file happens to have a line break exactly at buffer limit

The check does only for strlen(line) space and buf_printf will only use at most space -1 and not print the final character ('\n') in this corner. Since a missing \n only breaks certificates at the start and end marker, missing line breaks otherwise do not trigger this error.

---
 src/openvpn/buffer.h  | 5 ++++-
 src/openvpn/options.c | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)
src/openvpn/buffer.h +4 -1
index 5695f64..0dc511b 100644
@@ -329,7 +329,10 @@ has_digit (const unsigned char* src)
 }
 
 /*
- * printf append to a buffer with overflow check
+ * printf append to a buffer with overflow check,
+ * due to usage of vsnprintf, it will leave space for
+ * a final null character and thus use only
+ * capacity - 1
  */
 bool buf_printf (struct buffer *buf, const char *format, ...)
 #ifdef __GNUC__
src/openvpn/options.c +2 -1
index 93baa2b..737d9a2 100644
@@ -3712,7 +3712,7 @@ read_inline_file (struct in_src *is, const char *close_tag, struct gc_arena *gc)
 	  endtagfound = true;
 	  break;
 	}
-      if (!buf_safe (&buf, strlen(line)))
+      if (!buf_safe (&buf, strlen(line)+1))
 	{
 	  /* Increase buffer size */
 	  struct buffer buf2 = alloc_buf (buf.capacity * 2);

@@ -3722,6 +3722,7 @@ read_inline_file (struct in_src *is, const char *close_tag, struct gc_arena *gc)
 	  buf = buf2;
 	}
       buf_printf (&buf, "%s", line);
+
     }
   if (!endtagfound)
     msg (M_FATAL, "ERROR: Endtag %s missing", close_tag);
Signature · 1 line
1.9.1
#1 lore raw
Quoted 30 lines · Arne Schwabe

The check does only for strlen(line) space and buf_printf will only use at most space -1 and not print the final character ('\n') in this corner. Since a missing \n only breaks certificates at the start and end marker, missing line breaks otherwise do not trigger this error.

---
 src/openvpn/buffer.h  | 5 ++++-
 src/openvpn/options.c | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)
src/openvpn/buffer.h +4 -1
index 5695f64..0dc511b 100644
@@ -329,7 +329,10 @@ has_digit (const unsigned char* src)
 }
 
 /*
- * printf append to a buffer with overflow check
+ * printf append to a buffer with overflow check,
+ * due to usage of vsnprintf, it will leave space for
+ * a final null character and thus use only
+ * capacity - 1
  */
 bool buf_printf (struct buffer *buf, const char *format, ...)
 #ifdef __GNUC__
src/openvpn/options.c +0 -0
index 93baa2b..737d9a2 100644

src/openvpn/options.c read_inline_file (struct in_src *is, const char *close_tag, struct gc_arena *gc) · line 3718

Quoted 8 lines
src/openvpn/options.c +1 -1
@@ -3712,7 +3712,7 @@ read_inline_file (struct in_src *is, const char *close_tag, struct gc_arena *gc)
          endtagfound = true;
          break;
        }
-      if (!buf_safe (&buf, strlen(line)))
+      if (!buf_safe (&buf, strlen(line)+1))
        {
          /* Increase buffer size */
          struct buffer buf2 = alloc_buf (buf.capacity * 2);

ACK. Good find.

src/openvpn/options.c read_inline_file (struct in_src *is, const char *close_tag, struct gc_arena *gc) · line 3728

Quoted 8 lines
src/openvpn/options.c +1 -0
@@ -3722,6 +3722,7 @@ read_inline_file (struct in_src *is, const char *close_tag, struct gc_arena *gc)
          buf = buf2;
        }
       buf_printf (&buf, "%s", line);
+
     }
   if (!endtagfound)
     msg (M_FATAL, "ERROR: Endtag %s missing", close_tag);

Was the extra newline on purpose?

-Steffan

#2 lore raw
In reply to Arne Schwabe

Your patch has been applied to the master and release/2.3 branch.

I've removed the extra blank line that sneaked in.

commit d40cbf0e2601b35bfb1c0551c6f3907b5c5178ff (master) commit 57576475f5d6e5dd9cdb64fa0edcf283ce4a05c9 (release/2.3)

Author: Arne Schwabe
Date:   Mon Jul 27 17:33:11 2015 +0200
     Fix commit e473b7c if an inline file happens to have a line break exactly at buffer limit

     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/9956
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering