https://bugs.kde.org/show_bug.cgi?id=514094

Mark Wielaard <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #12 from Mark Wielaard <[email protected]> ---
Using the resolved_exename directly instead of going through do_syscall
(readlink) with cl_exec_fd is smart.
So we don't have a do_syscall invocation in the PRE handler. Nice.

But this looks dangerous:

+       HChar* out_name = (HChar*)ARG3;
+       SizeT res = VG_(strlen)(VG_(resolved_exename));
+       res = VG_MIN(res, ARG4);
+       VG_(strncpy)(out_name, VG_(resolved_exename), res);

We never check ARG3 (ARG2 for readlink) is fully writable, so may crash on the
strncpy.
I think we need a check like:

diff --git a/coregrind/m_syswrap/syswrap-generic.c
b/coregrind/m_syswrap/syswrap-generic.c
index 406e6960b0bb..aa4a0caf11f9 100644
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -5096,8 +5096,12 @@ PRE(sys_readlink)
          HChar* out_name = (HChar*)ARG2;
          SizeT res = VG_(strlen)(VG_(resolved_exename));
          res = VG_MIN(res, ARG3);
-         VG_(strncpy)(out_name, VG_(resolved_exename), res);
-         SET_STATUS_Success(res);
+         if (ML_(safe_to_deref)(out_name, res)) {
+            VG_(strncpy)(out_name, VG_(resolved_exename), res);
+            SET_STATUS_Success(res);
+         } else {
+            SET_STATUS_Failure(VKI_EFAULT);
+         }
          fuse_may_block = False;
       }
    }

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to