This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug libc/25219] New: improve out-of-bounds checking with GCC 10 attribute access

            Bug ID: 25219
           Summary: improve out-of-bounds checking with GCC 10 attribute
           Product: glibc
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: msebor at gmail dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

To help detect out-of-bounds accesses by third party functions, GCC 10 provides
a new function attribute called access.  It associates pointer parameters with
size parameters in function declarations so that actual arguments can be
checked in calls to the functions to determine if they are in bounds.  Those
that are not are diagnosed by GCC without changing the code (so unlike
_FORTIFY_SOURCE it doesn't abort at runtime).

GCC has had the ability for some time to detect these invalid accesses by
built-in functions and Glibc already has a mechanism to detect a subset of
buffer overflows by functions it defines and that GCC doesn't recognize as
built-ins.  But by relying on __builtin_object_size, the Glibc solution is
limited to constant sizes and offsets.  The detection enabled by the new
attribute doesn't have this limitation.

Making use of the new attribute in Glibc's APIs will significantly improve
GCC's ability to detect buffer overflows and other bugs in ordinary functions
(non-built-ins) defined by Glibc.  I have annotated a few of these functions
with the attribute as a proof of concept to see how this might look in
practice.  For instance, compiling the following example with GCC 10 and
unmodified Glibc and -D_FORTIFY_SOURCE=2 only the buffer overflow in function f
is diagnosed:

#include <string.h>
#include <unistd.h>

char a[32];

void f (int i)
  if (i < 27)

  strcpy (a + i, "123456");

int g (int fd, int i)
  if (i < 27)
    return -1;

  return read (fd, a + i, sizeof a);

In file included from ../include/bits/string_fortified.h:1,
                 from ../string/string.h:494,
                 from ../include/string.h:60,
                 from /build/tmp/t.c:1:
In function ‘strcpy’,
    inlined from ‘f’ at /build/tmp/t.c:11:3:
../string/bits/string_fortified.h:90:10: warning: ‘__builtin_memcpy’ forming
offset [32, 33] is out of the bounds [0, 32] of object ‘a’ with type ‘char[32]’
   90 |   return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/tmp/t.c: In function ‘f’:
/build/tmp/t.c:4:6: note: ‘a’ declared here
    4 | char a[32];
      |      ^

Applying the patch below lets the overflow in function g() also be diagnosed:

In function ‘read’,
    inlined from ‘g’ at /build/tmp/t.c:19:10:
../posix/bits/unistd.h:45:10: warning: ‘__read_alias’ writing 32 bytes into a
region of size 5 overflows the destination [-Wstringop-overflow=]
   45 |   return __read_alias (__fd, __buf, __nbytes);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../include/sys/cdefs.h:3,
                 from ../include/features.h:465,
                 from ../posix/sys/types.h:25,
                 from ../include/sys/types.h:1,
                 from ../include/string.h:6,
                 from /build/tmp/t.c:1:
../posix/bits/unistd.h: In function ‘g’:
../posix/bits/unistd.h:25:28: note: in a call to function ‘__read_alias’
declared with attribute ‘write_only (2, 3)’
   25 | extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
      |                            ^~~~~~~~~~~~
../misc/sys/cdefs.h:174:41: note: in definition of macro ‘__REDIRECT’
  174 | # define __REDIRECT(name, proto, alias) name proto __asm__ (__ASMNAME
      |                                         ^~~~

diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index abcb0d5e3c..268d0ed1f2 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -522,4 +522,19 @@

+#if __GNUC_PREREQ (10, 0)
+/* Denotes a function pointer argument can be used to access an array
+   of N elements according to access mode.  */
+#  define __rdonly(...)  \
+  __attribute__ ((__access__ (__read_only__, __VA_ARGS__)))
+#  define __rdwr(...)  \
+  __attribute__ ((__access__ (__read_write__, __VA_ARGS__)))
+#  define __wronly(...) \
+  __attribute__ ((__access__ (__write_only__, __VA_ARGS__)))
+#  define __rdonly(...)
+#  define __rdwr(...)
+#  define __wronly(...)
 #endif  /* sys/cdefs.h */
diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
index 103095b8e4..8e4b0f084e 100644
--- a/posix/bits/unistd.h
+++ b/posix/bits/unistd.h
@@ -21,9 +21,10 @@

 extern ssize_t __read_chk (int __fd, void *__buf, size_t __nbytes,
-                          size_t __buflen) __wur;
+                          size_t __buflen) __wur __wronly (2, 3);
 extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
-                                         size_t __nbytes), read) __wur;
+                                         size_t __nbytes), read)
+  __wur __wronly (2, 3);
 extern ssize_t __REDIRECT (__read_chk_warn,
                           (int __fd, void *__buf, size_t __nbytes,
                            size_t __buflen), __read_chk)

You are receiving this mail because:
You are on the CC list for the bug.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]