Bug 12685 - fopen doesn't honor last byte of valid modes
Summary: fopen doesn't honor last byte of valid modes
Status: NEW
Alias: None
Product: glibc
Classification: Unclassified
Component: stdio (show other bugs)
Version: 2.13
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-20 00:46 UTC by Eric Blake
Modified: 2016-05-16 17:11 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Blake 2011-04-20 00:46:39 UTC
$ cat foo.c
#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>

int main (void)
{
  FILE *f = fopen ("/dev/null", "rb+cmxe");
  if (!f) exit (1);
  int fd = fileno (f);
  if (fd < 0) exit (2);
  int mode = fcntl (fd, F_GETFD);
  if (mode < 0) exit (3);
  return !(mode & FD_CLOEXEC);
}
$ ./foo; echo $?
1

Oops - an off-by-one error in fileops.c ended up ignoring 'e'.  The same goes for any other valid byte that doesn't appear until mode[6].

diff --git i/libio/fileops.c w/libio/fileops.c
index 4698953..ee6ce13 100644
--- i/libio/fileops.c
+++ w/libio/fileops.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1993, 1995, 1997-2005, 2006, 2007, 2008, 2009
+/* Copyright (C) 1993, 1995, 1997-2005, 2006, 2007, 2008, 2009, 2011
    Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Written by Per Bothner <bothner@cygnus.com>.
@@ -290,7 +290,7 @@ _IO_new_file_fopen (fp, filename, mode, is32not64)
 #ifdef _LIBC
   last_recognized = mode;
 #endif
-  for (i = 1; i < 6; ++i)
+  for (i = 1; i < 7; ++i)
     {
       switch (*++mode)
 	{
Comment 1 Ulrich Drepper 2011-04-23 03:41:46 UTC
The patch didn't apply.  I did it manually.
Comment 2 Michael Kerrisk 2012-04-22 01:58:14 UTC
Actually, is there any reason at all to limit the length of the string? Whay not parse until NUL? Given that "mode" may be a constructed string, or that other flags may be added in the future, it seems that arbitrarily limiting the length of the string to some number of bytes is just setting up both glibc and userspace bugs for the future.
Comment 3 Michael Kerrisk 2012-04-22 02:01:46 UTC
A similar remark applies for fdopen(), which uses different code for parsing 'mode'.
Comment 4 Michael Kerrisk 2012-04-22 02:11:48 UTC
Sorry, my original comment should have read:

Actually, is there any reason at all to limit the length of the string? Why
not parse until NUL *or comma*? Given that "mode" may be a constructed string, or that other flags may be added in the future, it seems that arbitrarily limiting the length of the string to some number of bytes is just setting up both glibc and userspace bugs for the future.
Comment 5 OndrejBilka 2013-05-09 15:13:53 UTC
Could you specify why do you want comma as terminator?
Comment 6 Rich Felker 2013-05-09 16:08:22 UTC
I believe it's to ignore some non-standard syntax certain implementations use for binding a non-default character encoding at open time. I recall seeing such a usage before but I can't think where.