diff mbox

Fix BZ #18757 (fmemopen doesn't fail with invalid modes)

Message ID CALoOobOhaK-1ZU4yJL7UZdgH1wqT-_oQ1Kx4Qhe8ENdG-=_yfg@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov Aug. 9, 2015, 10:28 p.m. UTC 
Greetings,

Attached patch adds a test for invalid modes in fmemopen, and fixes BZ #18757.
Tested on Linux/x86_64, no new failures.

Thanks,

2015-08-09  Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #18757]
        * libio/fmemopen.c (valid_mode_p): New function.
        (__fmemopen): Call it.
        * libio/test-fmemopen.c (do_bz18757): New test.

Comments

Mike Frysinger Aug. 10, 2015, 2:36 a.m. UTC | #1 
On 09 Aug 2015 15:28, Paul Pluzhnikov wrote:
> +/* Check valid open mode.  Only "(r|w|a)\+?" are valid.  */

this is not what the documentation states:
https://www.gnu.org/software/libc/manual/html_node/String-Streams.html
	"The argument opentype is the same as in fopen"
https://www.gnu.org/software/libc/manual/html_node/Opening-Streams.html
	there are various GNU extensions here, and the standard 'b' flag

it's also not what POSIX requires:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fmemopen.html
	it mentions the 'b' flag, and lists everything else as 'behavior 
	is undefined'

if you look at _IO_new_file_fopen in libio/fileops.c, you'll see the parsing
logic for mode is generally forgiving of unknown modes.  this is a good thing
as it means when glibc adds new extensions (like 'e'), you don't have to do a
version check at runtime before passing it in ... you can just do:
	fopen ("foo", "we");
for new versions, it does the right thing, and for old versions, it's ignored.

so the only thing we should check is whether the first byte is [rwa].  and it
seems to me that _IO_fopencookie (which fmemopen tails into) already does just
that for us.
-mike
diff mbox

Patch

diff --git a/libio/fmemopen.c b/libio/fmemopen.c
index 3ab3e8d..2cf45c0 100644
--- a/libio/fmemopen.c
+++ b/libio/fmemopen.c
@@ -144,12 +144,30 @@  fmemopen_close (void *cookie)
 }
 
 
+/* Check valid open mode.  Only "(r|w|a)\+?" are valid.  */
+static int
+valid_mode_p (const char *mode)
+{
+  if (mode[0] != 'r' && mode[0] != 'w' && mode[0] != 'a')
+    return 0;
+  if (mode[1] == '\0' || (mode[1] == '+' && mode[2] == '\0'))
+    return 1;
+
+  return 0;
+}
+
 FILE *
 __fmemopen (void *buf, size_t len, const char *mode)
 {
   cookie_io_functions_t iof;
   fmemopen_cookie_t *c;
 
+  if (!valid_mode_p (mode))
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+
   c = (fmemopen_cookie_t *) calloc (sizeof (fmemopen_cookie_t), 1);
   if (c == NULL)
     return NULL;
diff --git a/libio/test-fmemopen.c b/libio/test-fmemopen.c
index 63ca89f..7c6b4e5 100644
--- a/libio/test-fmemopen.c
+++ b/libio/test-fmemopen.c
@@ -24,6 +24,68 @@  static char buffer[] = "foobar";
 #include <errno.h>
 
 static int
+do_bz18757_one (const char *mode)
+{
+  char c = 0;
+  FILE *stream;
+
+  errno = 0;
+  stream = fmemopen (&c, 1, mode);
+  if (stream == NULL)
+    {
+      if (errno == EINVAL)
+        return 1;
+
+      printf ("fmemopen invalid mode %s: %m\n", mode);
+      return 2;
+    }
+
+  fclose (stream);
+  return 0;
+}
+
+static int
+do_bz18757 (void)
+{
+  int ret = 0;
+  int j, k;
+  const char valid[] = "rwa";
+  const char invalid[][4] = {"?", "rw", "r++", "+w", "a+r"};
+
+  /* Test all valid modes.  */
+  for (j = 0; j < strlen(valid); ++j)
+    for (k = 0; k < 2; ++k)
+      {
+	char mode[3];
+
+	mode[0] = valid[j];
+        mode[1] = k ? '\0' : '+';
+        mode[2] = '\0';
+
+        if (do_bz18757_one (mode))
+	  {
+	    printf ("fmemopen rejected '%s'\n", mode);
+	    ret += 1;
+	  }
+      }
+
+  /* Test some invalid modes.  */
+  for (j = 0; j < sizeof (invalid) / sizeof (invalid[0][0]); ++j)
+    {
+      const char *mode = invalid[j];
+      int rc = do_bz18757_one (mode);
+
+      if (rc != 1)
+	{
+	  printf ("mode: %s, expected 1, got %d\n", mode, rc);
+	  ret += 1;
+	}
+    }
+
+  return ret;
+}
+
+static int
 do_test (void)
 {
   int ch;
@@ -44,7 +106,7 @@  do_test (void)
 
   fclose (stream);
 
-  return ret;
+  return ret + do_bz18757 ();
 }
 
 #define TEST_FUNCTION do_test ()