[Valgrind-developers] Valgrind: r16364 - in /trunk: NEWS coregrind/m_commandline.c docs/xml/manual-core.xml

[<sv...@va...>]

Author: iraisr
Date: Thu May 11 18:00:17 2017
New Revision: 16364

Log:
Fix a problem when reading .valgrindrc and it is a directory instead of a file.
Fixes BZ#362223.

Modified:
    trunk/NEWS
    trunk/coregrind/m_commandline.c
    trunk/docs/xml/manual-core.xml

Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Thu May 11 18:00:17 2017
@@ -112,6 +112,7 @@
 358697  valgrind.h: Some code remains even when defining NVALGRIND
 359202  Add musl libc configure/compile
 360429  unhandled ioctl 0x530d with no size/direction hints (CDROMREADMODE1)
+362223  assertion failed when .valgrindrc is a directory instead of a file
 367942  Segfault vgPlain_do_sys_sigaction (m_signals.c:1138)
 368863  WARNING: unhandled arm64-linux syscall: 100 (get_robust_list)
 368865  WARNING: unhandled arm64-linux syscall: 272 (kcmp)

Modified: trunk/coregrind/m_commandline.c
==============================================================================
--- trunk/coregrind/m_commandline.c (original)
+++ trunk/coregrind/m_commandline.c Thu May 11 18:00:17 2017
@@ -55,8 +55,6 @@
 
 static HChar* read_dot_valgrindrc ( const HChar* dir )
 {
-   Int    n;
-   SysRes fd;
    struct vg_stat stat_buf;
    HChar* f_clo = NULL;
    const  HChar dot_valgrindrc[] = ".valgrindrc";
@@ -66,15 +64,18 @@
    HChar filename[VG_(strlen)(dir) + 1 + VG_(strlen)(dot_valgrindrc) + 1];
    VG_(sprintf)(filename, "%s/%s", dir, dot_valgrindrc);
 
-   fd = VG_(open)(filename, 0, VKI_S_IRUSR);
+   SysRes fd = VG_(open)(filename, 0, VKI_S_IRUSR);
    if ( !sr_isError(fd) ) {
       Int res = VG_(fstat)( sr_Res(fd), &stat_buf );
-      // Ignore if not owned by current user or world writeable (CVE-2008-4865)
-      if (!res && stat_buf.uid == VG_(geteuid)()
-          && (!(stat_buf.mode & VKI_S_IWOTH))) {
+      /* Ignore if not owned by the current user, or is not a regular file,
+         or is world writeable (CVE-2008-4865). */
+      if (res == 0
+          && stat_buf.uid == VG_(geteuid)()
+          && (stat_buf.mode & VKI_S_IFREG)
+          && !(stat_buf.mode & VKI_S_IWOTH)) {
          if ( stat_buf.size > 0 ) {
             f_clo = VG_(malloc)("commandline.rdv.1", stat_buf.size+1);
-            n = VG_(read)(sr_Res(fd), f_clo, stat_buf.size);
+            Int n = VG_(read)(sr_Res(fd), f_clo, stat_buf.size);
             if (n == -1) n = 0;
             vg_assert(n >= 0 && n <= stat_buf.size+1);
             f_clo[n] = '\0';
@@ -82,8 +83,9 @@
       }
       else
          VG_(message)(Vg_UserMsg,
-               "%s was not read as it is either world writeable or not "
-               "owned by the current user\n", filename);
+            "%s was not read as it is either not a regular file,\n"
+            "    or is world writeable, or is not owned by the current user.\n",
+            filename);
 
       VG_(close)(sr_Res(fd));
    }

Modified: trunk/docs/xml/manual-core.xml
==============================================================================
--- trunk/docs/xml/manual-core.xml (original)
+++ trunk/docs/xml/manual-core.xml Thu May 11 18:00:17 2017
@@ -2567,8 +2567,8 @@
 </para>
 
 <para>Please note that the <computeroutput>./.valgrindrc</computeroutput>
-file is ignored if it is marked as world writeable or not owned 
-by the current user. This is because the
+file is ignored if it is not a regular file, or is marked as world writeable,
+or is not owned by the current user. This is because the
 <computeroutput>./.valgrindrc</computeroutput> can contain options that are
 potentially harmful or can be used by a local attacker to execute code under
 your user account.