[Mono-bugs] [Bug 82678][Maj] New - Mono_Posix_Syscall_readdir_r causes stack corruption in readdir_r, resulting in crash

[bugzilla-daemon at bugzilla.ximian.com]

Please do not reply to this email- if you want to comment on the bug, go to the
URL shown below and enter your comments there.

Changed by trisk+ximian at acm.jhu.edu.

http://bugzilla.ximian.com/show_bug.cgi?id=82678

--- shadow/82678	2007-09-03 15:40:30.000000000 -0400
+++ shadow/82678.tmp.32274	2007-09-03 15:40:30.000000000 -0400
@@ -0,0 +1,103 @@
+Bug#: 82678
+Product: Mono: Class Libraries
+Version: 1.2
+OS: 
+OS Details: 
+Status: NEW   
+Resolution: 
+Severity: 002 Two hours
+Priority: Major
+Component: Mono.POSIX
+AssignedTo: miguel at ximian.com                            
+ReportedBy: trisk+ximian at acm.jhu.edu               
+QAContact: mono-bugs at ximian.com
+TargetMilestone: ---
+URL: 
+Cc: 
+Summary: Mono_Posix_Syscall_readdir_r causes stack corruption in readdir_r, resulting in crash
+
+Description of Problem:
+Mono_Posix_Syscall_readdir_r (support/dirent.c) allocates a struct
+dirent-sized buffer on the stack to store the readdir_r results in. 
+
+However, readdir_r expects to have storage space for up to PATH_MAX (on
+POSIX) characters, which is not guaranteed by struct dirent's d_name
+member. So it can spew a filename over the rest of the stack if the
+filename goes past the of dirent end (this is easy where the sizeof(d_name)
+is 1).
+
+This results in a crash.
+
+
+There is an additional bug in the return value check:
+
+ if (r == 0 && result != NULL) {
+
+result is type dirent ** and was passed by value to readdir_r, thus it is
+never modified. the correct check is:
+
+ if (r == 0 && *result != NULL) {
+
+Steps to reproduce the problem:
+1. Build Mono on a platform where PATH_MAX is larger than sizeof(struct dirent)
+2. Call Mono.Unix.Native.Syscall.readdir_r directly or through
+Mono.Unix.UnixDirectoryInfo.GetFileSystemEntries on a directory with some
+long-ish filenames
+3. Watch the lovely crash in memset() to a pointer which actually contains
+ characters of a filename
+
+The example here will using the "Import Folder" function of Banshee on a
+Solaris system.
+
+Actual Results:
+Stacktrace:
+
+  at (wrapper managed-to-native) Mono.Unix.Native.Syscall.sys_readdir_r
+(intptr,Mono.Unix.Native.Syscall/_Dirent&,intptr&) <0x00004>
+  at (wrapper managed-to-native) Mono.Unix.Native.Syscall.sys_readdir_r
+(intptr,Mono.Unix.Native.Syscall/_Dirent&,intptr&) <0xffffffff>
+  at Mono.Unix.Native.Syscall.readdir_r
+(intptr,Mono.Unix.Native.Dirent,intptr&) <0x0005c>
+  at Mono.Unix.UnixDirectoryInfo.GetEntries (intptr) <0x0007a>
+  at Mono.Unix.UnixDirectoryInfo.GetEntries () <0x0004b>
+  at Mono.Unix.UnixDirectoryInfo.GetFileSystemEntries () <0x0000d>
+  at <>c__CompilerGenerated31.MoveNext () <0x0005f>
+  at Banshee.Base.ImportManager.ScanForFiles (string) <0x00236>
+  at <>c__CompilerGenerated63.<>c__AnonymousMethod190 () <0x0005e>
+  at (wrapper delegate-invoke) System.MulticastDelegate.invoke_void ()
+<0xffffffff>
+  at (wrapper runtime-invoke) System.Object.runtime_invoke_void
+(object,intptr,intptr,intptr) <0xffffffff>
+
+Native stacktrace:
+
+        /usr/mono/bin/mono'mono_handle_native_sigsegv+0xd6 [0x81aaa38]
+        /usr/mono/bin/mono'sigsegv_signal_handler+0xb5 [0x818e79c]
+        /lib/libc.so.1'__sighndlr+0xf [0xfef2dcbf]
+        /lib/libc.so.1'call_user_handler+0x2b8 [0xfef22beb]
+        [0x0]
+
+Debug info from gdb:
+
+
+
+=================================================================
+Got a SIGSEGV while executing native code. This usually indicates
+a fatal error in the mono runtime or one of the native libraries 
+used by your application.
+=================================================================
+
+
+
+Expected Results:
+The import actually succeeds.
+
+How often does this happen?
+Always.
+
+Additional Information:
+I am attaching a patch that fixes the problems described.
+
+http://womble.decadentplace.org.uk/readdir_r-advisory.html describes a
+similar, but more subtle problem arising from use of readdir_r, thus the
+fix should use PATH_MAX (which I believe is POSIX mandated) and not pathconf().