Bug 52627

Summary: Segmentation fault in org.apache.tomcat.jni.File.infoGet() native method
Product: Tomcat Native Reporter: Ilya Maykov <ivmaykov>
Component: LibraryAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: minor    
Priority: P2    
Version: 1.1.22   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Source code for the repro case
Proposed patch

Description Ilya Maykov 2012-02-08 22:21:15 UTC
Created attachment 28291 [details]
Source code for the repro case

There is a potential segfault in the tomcat native wrappers around apr_file_info_get() (which themselves probably wrap the fstat() and stat() syscalls).

I've attached a simple java file that reproduces this 100% of the time on my OS X 10.7 system with tomcat-native-1.1.22. I'm not sure if infoGet() is actually used in the tomcat server codebase, I stumbled upon the bug in my own project that uses the tomcat native library's JNI wrappers around Apache Portable Runtime.

Repro instructions:

1) Download and build tomcat-native (I think I got the source from https://github.com/apache/tomcat-native and followed the instructions). You may need to install libapr (apache portable runtime) through apt-get or port or whatever package manager you use.

2) Download the TomcatNativeCrash.java attachment
3) Copy tomcat-native-*-dev.jar to the same directory as the attachment.
4) Copy the libtcnative library files for your architecture to the same directory as the attachment (they get built into tomcat-native/jni/native/.libs)
3) Build the repro case with:

javac -classpath ./tomcat-native*.jar:. TomcatNativeCrash.java

4) Run the repro case with:

java -Djava.library.path=. -classpath ./tomcat-native*.jar:. ./TomcatNativeCrash.java

The repro just opens a file descriptor to the file named by the first argument (its own source in the example), creates a FileInfo structure, and tries to fstat it via org.apache.tomcat.jni.File.infoGet() 100,000 times. This segfaults for me every single time.

I've coded up a fix and submitted it as a pull request on github: https://github.com/apache/tomcat-native/pull/1
Comment 1 Mark Thomas 2012-02-08 22:28:22 UTC
For the record, Tomcat doesn't use that method.
Comment 2 Ilya Maykov 2012-02-08 22:36:42 UTC
Right. But the tomcat-native library is open-source so it's possible to use it in any Java (Scala, Clojure, Groovy, ...) project that's license-compatible with the Apache license, which is how I found the bug. I'm ok with keeping my own fork and using it in our project, but just figured I would report the bug and contribute my bugfix back to you guys :)
Comment 3 Ilya Maykov 2012-02-08 22:46:04 UTC
Created attachment 28292 [details]
Proposed patch

Here is my proposed bugfix, which uses memset() to zero-out the stack-allocated apr_finfo_t structure before using it. This patch makes the repro case (attached earlier) stop crashing.
Comment 4 Christopher Schultz 2013-05-24 20:56:49 UTC
Can you give some more information about the crash? For example, build tcnative with debugging symbols and then post the JVM crash dump?

I don't see why zeroing-out a apr_finfo_t structure before calling apr_file_info_get would fix anything, since that structure is only used to deposit information, not to read information.

I think the real problem is that the (tcnative) fill_finfo function isn't sensitive to which fields in the apr_finfo_t structure are actually valid, so it does stupid things like trying to create UTF strings out of potentially invalid (char *) values.
Comment 5 Ilya Maykov 2013-05-25 02:23:53 UTC
It's been a while since I reported this bug and I've had my work machine upgraded since then, so I probably don't have the necessary environment to reproduce this quickly. But from what I remember, yes the bug was in some native function trying interpret garbage values in the stack-allocated apr_finfo_t struct as char* pointers ... I think ... like I said it's been a while.
Comment 6 Mark Thomas 2017-01-31 19:58:01 UTC
I couldn't repeat this on a current OSX but I've applied a variation of the patch for 1.2.11. I also turned the repro case into a JUnit test case.