Closed Bug 183092 Opened 17 years ago Closed 17 years ago
Buffer overrun calling Java from JS
472 bytes, text/plain
1.28 KB, patch
|Details | Diff | Splinter Review|
636 bytes, patch
|Details | Diff | Splinter Review|
from Georgi Guninski http://www.guninski.com There is a crash (signal 11, SIGSEGV) when passing long (tested with >130000) arguments to a lot of java objects from js. Can't debug the javavm for unknown reasons, so can't tell if it is exploitable but may be. Attached is java3.pl - perl cgi which demonstrates the problem with ----- $a="g"x130000; ... x=new java.net.Socket("$a",1234); ---- The console does not give chance to debug and reports the message: .... Unexpected Signal : 11 occurred at PC=0x401c1b38 Function name=utf8_length__7UNICODEPUsi Library=/opt/netscape7/plugins/java2/lib/i386/client/libjvm.so ....
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.3beta
Assignee: mstoltz → nisheeth
Status: ASSIGNED → NEW
I can't understand the test case in comment #1 properly: >$a="g"x130000; > >... >x=new java.net.Socket("$a",1234); How does this test case end up "passing long (tested with >130000) arguments to a lot of java objects from js"? Mitch/Georgi, please clarify. Thanks!
Nisheeth, this is a snippet from perl code. Will attach the perl cgi. In order to test it, you should make the perl executable and place it in a cgi directory. Another possibility is to do save the output in a html file so: perl java3.html > java3.html and then open java3.html. If you don't have perl, I will attach the raw html. This crashes mozilla1.3a with jre1.3.1 on linux. Can't make java work on a nightly build - it complains about an unresolved symbol.
This cgi should be made executable and placed in cgi directory, or the output from it should be placed in a .html file and then opened.
Typo in comment #3. It should read perl java3.pl > java3.html A short explanation. In perl it is very easy to create long strings. $a="g"x130000; means assign to the variable $a the string "g" concatenated 130000 times.
nisheeth, we *really* should get this fixed for 1.3final. It looks like a crash in the JVM but for a workaround we could just limit the maximum string length in Liveconnect.
ok, I set up the cgi internally and accessed it on Windows. I did not hang but I did not see the alert box that is supposed to pop up (right, Georgi?). I got the following security error on the java console: java.security.PrivilegedActionException: java.lang.reflect.InvocationTargetException at java.security.AccessController.doPrivileged(Native Method) at sun.plugin.liveconnect.SecureInvocation$1.run(Unknown Source) at java.security.AccessController.doPrivileged(Native Method) at sun.plugin.liveconnect.SecureInvocation.ConstructObject(Unknown Source) Caused by: java.lang.reflect.InvocationTargetException at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(Unknown Source) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(Unknown Source) at java.lang.reflect.Constructor.newInstance(Unknown Source) at sun.plugin.liveconnect.PrivilegedConstructObjectAction.run(Unknown Source) ... 4 more Caused by: java.security.AccessControlException: access denied (java.net.SocketPermission resolve) at java.security.AccessControlContext.checkPermission(Unknown Source) at java.security.AccessController.checkPermission(Unknown Source) at java.lang.SecurityManager.checkPermission(Unknown Source) at java.lang.SecurityManager.checkConnect(Unknown Source) at java.net.InetAddress.getAllByName0(Unknown Source) at java.net.InetAddress.getAllByName0(Unknown Source) at java.net.InetAddress.getAllByName(Unknown Source) at java.net.InetAddress.getByName(Unknown Source) at java.net.InetSocketAddress.<init>(Unknown Source) at java.net.Socket.<init>(Unknown Source) ... 9 more I'm going to test this on Linux next...
Nisheeth, not seeing the alert and getting java security exception is normal behavior and is not a bug.
I just filed bug 192657 for a problem I ran into with testing this on the latest debug build. For some reason, the Java plugin isn't loading at Mozilla startup. Will follow up on this tomorrow...
ok, so the problem I'm running into is described in bug 116444. My mozilla is compiled with gcc 3.0 which does name mangling of C++ methods differently from gcc 2.9x.x. The java plugin was built with the gcc 2.9x.x version and thus isn't binary compatible with my Mozilla build. I'm installing the older version of gcc and re-compiling Mozilla...
Setting platform to Linux only. I am able to reproduce a hang with my new debug build compiled with gcc 2.96...
OS: Windows 2000 → Linux
I don't see a seg fault. But, I hang on Linux with the following stack trace: #0 0x42073b2d in _int_malloc () from /lib/i686/libc.so.6 #1 0x42073155 in malloc () from /lib/i686/libc.so.6 #2 0x4045200d in __builtin_new (sz=32) at ../../gcc/cp/new1.cc:-1 #3 0x41fad9c0 in JVM_GetJSSecurityContext () at jvmmgr.cpp:519 #4 0x41fbbab9 in ProxyJNIEnv::getContext (this=0x88bbf98) at ProxyJNI.cpp:253 #5 0x41fbbb3b in ProxyJNIEnv::InvokeMethod (env=0x88bbf98, obj=0x80f5e24, method=0x88e4828, args=0x0) at ProxyJNI.cpp:532 #6 0x41fbbc00 in ProxyJNIEnv::InvokeMethod (env=0x88bbf98, obj=0x80f5e24, method=0x88e4828, args=0xbfffdb3c) at ProxyJNI.cpp:543 #7 0x41fb7de5 in ProxyJNIEnv::CallObjectMethod (env=0x88bbf98, obj=0x80f5e24, methodID=0x88e4828) at ProxyJNI.cpp:604 #8 0x41fd7959 in jsj_GetJavaClassName (cx=0x8716710, jEnv=0x88bbf98, java_class=0x80f5e24) at jsj_class.c:77 #9 0x41fde93d in invoke_java_constructor (cx=0x8716710, jsj_env=0x88bcda8, java_class=0x80f5e24, method=0x88bd428, argv=0x88cb8d0, vp=0xbfffdd70) at jsj_method.c:1590 #10 0x41fdea83 in invoke_overloaded_java_constructor (cx=0x8716710, jsj_env=0x88bcda8, member=0x88bd2f8, class_descriptor=0x88e44a8, argc=2, argv=0x88cb8d0, vp=0xbfffdd70) at jsj_method.c:1633 #11 0x41fdeb95 in java_constructor_wrapper (cx=0x8716710, jsj_env=0x88bcda8, member_descriptor=0x88bd2f8, class_descriptor=0x88e44a8, argc=2, argv=0x88cb8d0, vp=0xbfffdd70) at jsj_method.c:1672 #12 0x41fdec6a in jsj_JavaConstructorWrapper (cx=0x8716710, obj=0x88b4cf0, argc=2, argv=0x88cb8d0, vp=0xbfffdd70) at jsj_method.c:1696 #13 0x4005d600 in js_Invoke (cx=0x8716710, argc=2, flags=1) at jsinterp.c:839 #14 0x4006723f in js_Interpret (cx=0x8716710, result=0xbfffe3b0) at jsinterp.c:2415 Also, the console displays the following: WARNING: CreateSecureEnv OK, file ProxyJNI.cpp, line 1679 Allocating more space for read msg 260008 **************** SERVER ERROR ************** ConstructJavaObject failed **************** ************ **************
Status: NEW → ASSIGNED
Probably this is closely related to bug 175358. In both cases the heap seems corrupted.
Running with valgrind (see http://devel-home.kde.org/~sewardj/), I see the following before a crash: ==20566== Invalid write of size 4 ==20566== at 0x45A0EFBE: ??? ==20566== by 0x45A07103: ??? ==20566== by 0x41189115: JavaCalls::call_helper(JavaValue *, methodHandle *, JavaCallArguments *, Thread *) (in /usr/java/j2re1.4.1_01/lib/i386/client/libjvm.so) ==20566== by 0x41223F44: os::os_exception_wrapper(void (*)(JavaValue *, methodHandle *, JavaCallArguments *, Thread *), JavaValue *, methodHandle *, JavaCallArguments *, Thread *) (in /usr/java/j2re1.4.1_01/lib/i386/client/libjvm.so) ==20566== Address 0xBFFF5CA4 is not stack'd, malloc'd or free'd ... the above invalid write message repeats ~26 times after which we crash. Also, either Mozilla or the JVM prints out the following message to the console: "Fatal: Stack size too small. Use 'java -Xss' to increase default stack size." Definitely seems like a buffer overflow to me somewhere in the JVM. If we want to workaround this by limiting buffer sizes in LiveConnect, how do we determine the limit. Where is the best place to check for the buffer? I think we need LiveConnect/OJI owners to pitch in with their expertise. CCing Joshua, the OJI owner, Roger, the Liveconnect owner, and Phil, the LiveConnect QA contact.
Looks like this is a dup of Bug 175358 the testcases are almost the same except for the size of the string?
I reproduced this bug by using mozilla 1.3b (built by gcc3.x) + jre 1.4.2 (built by gcc3.x) on linux. I will try to reproduce this bug by using mozilla nightly build and jre 1.4.2 nightly build. It seems out of bound exception (parameter is too long). It seems that "ConstructJavaObject failed" and "CSecureJNI2_CallMethod: Bad java method" don't lead to crash. I can't reproduce this bug on Solaris.
I can reproduce this bug by using mozilla nightly build and jre 1.4.2 nightly build. It seems problem in JPI. I am checking it. root@dhcp-cbjs05-217-4 mozilla]# ./mozilla Allocating more space for read msg 260008 **************** SERVER ERROR ************** ConstructJavaObject failed **************** ************ ************** **************** SERVER ERROR ************** CSecureJNI2_CallMethod: Bad java method **************** ************ ************** Unexpected Signal : 11 occurred at PC=0x4032D210 Function=(null)+0x4032D210 Library=/workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/client/libjvm.so NOTE: We are unable to locate the function name symbol for the error just occurred. Please refer to release documentation for possible reason and solutions. Current Java thread: at sun.plugin.navig.motif.AThread.handleRequest(Native Method) at sun.plugin.navig.motif.AThread.JNIHandleLoop(AThread.java:35) at sun.plugin.navig.motif.AThread.run(AThread.java:27) Dynamic libraries: 08048000-0804b000 r-xp 00000000 03:02 3081189 /workspace/java/bundles/j2sdk1.4.2/jre/bin/java_vm 0804b000-0804c000 rw-p 00002000 03:02 3081189 /workspace/java/bundles/j2sdk1.4.2/jre/bin/java_vm 40000000-40012000 r-xp 00000000 03:02 884740 /lib/ld-2.2.93.so 40012000-40013000 rw-p 00012000 03:02 884740 /lib/ld-2.2.93.so 40013000-4001b000 r-xp 00000000 03:02 2606257 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/native_threads/libhpi.so 4001b000-4001c000 rw-p 00007000 03:02 2606257 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/native_threads/libhpi.so 4001c000-40025000 r-xp 00000000 03:02 884773 /lib/libnss_files-2.2.93.so 40025000-40026000 rw-p 00008000 03:02 884773 /lib/libnss_files-2.2.93.so 40026000-4002a000 rw-s 00000000 03:02 3081179 /tmp/hsperfdata_root/2105 4002a000-4002d000 r--s 00000000 03:02 3081192 /workspace/java/bundles/j2sdk1.4.2/jre/lib/ext/dnsns.jar 4002d000-4002e000 r-xp 00000000 03:02 295786 /usr/X11R6/lib/X11/locale/lib/common/xlcUTF8Load.so.2 4002e000-4002f000 rw-p 00000000 03:02 295786 /usr/X11R6/lib/X11/locale/lib/common/xlcUTF8Load.so.2 4002f000-4003c000 r-xp 00000000 03:02 901128 /lib/i686/libpthread-0.10.so 4003c000-4003f000 rw-p 0000d000 03:02 901128 /lib/i686/libpthread-0.10.so 40060000-40062000 r-xp 00000000 03:02 884753 /lib/libdl-2.2.93.so 40062000-40063000 rw-p 00001000 03:02 884753 /lib/libdl-2.2.93.so 40063000-4046f000 r-xp 00000000 03:02 1033257 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/client/libjvm.so 4046f000-4048a000 rw-p 0040b000 03:02 1033257 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/client/libjvm.so 4049c000-404ae000 r-xp 00000000 03:02 884757 /lib/libnsl-2.2.93.so 404ae000-404af000 rw-p 00012000 03:02 884757 /lib/libnsl-2.2.93.so 404b1000-404d2000 r-xp 00000000 03:02 901126 /lib/i686/libm-2.2.93.so 404d2000-404d3000 rw-p 00021000 03:02 901126 /lib/i686/libm-2.2.93.so 404d3000-404e3000 r-xp 00000000 03:02 2606264 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libverify.so 404e3000-404e5000 rw-p 0000f000 03:02 2606264 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libverify.so 404e5000-40505000 r-xp 00000000 03:02 2606265 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libjava.so 40505000-40507000 rw-p 0001f000 03:02 2606265 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libjava.so 40507000-4051b000 r-xp 00000000 03:02 2606267 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libzip.so 4051b000-4051e000 rw-p 00013000 03:02 2606267 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libzip.so 4051e000-41e9f000 r--s 00000000 03:02 2606318 /workspace/java/bundles/j2sdk1.4.2/jre/lib/rt.jar 41ee9000-41eff000 r--s 00000000 03:02 2606289 /workspace/java/bundles/j2sdk1.4.2/jre/lib/sunrsasign.jar 41eff000-41fd9000 r--s 00000000 03:02 2606315 /workspace/java/bundles/j2sdk1.4.2/jre/lib/jsse.jar 41fd9000-41fea000 r--s 00000000 03:02 2606290 /workspace/java/bundles/j2sdk1.4.2/jre/lib/jce.jar 41fea000-41ff7000 r--s 00000000 03:02 3081194 /workspace/java/bundles/j2sdk1.4.2/jre/lib/ext/ldapsec.jar 41ff7000-41ffe000 r-xp 00000000 03:02 3670056 /usr/X11R6/lib/libXp.so.6.2 41ffe000-41fff000 rw-p 00006000 03:02 3670056 /usr/X11R6/lib/libXp.so.6.2 42000000-42126000 r-xp 00000000 03:02 901124 /lib/i686/libc-2.2.93.so 42126000-4212b000 rw-p 00126000 03:02 901124 /lib/i686/libc-2.2.93.so 4212f000-42688000 r--s 00000000 03:02 2606316 /workspace/java/bundles/j2sdk1.4.2/jre/lib/charsets.jar 42688000-42842000 r--s 00000000 03:02 2606317 /workspace/java/bundles/j2sdk1.4.2/jre/lib/plugin.jar 448ea000-448ee000 r-xp 00000000 03:02 3670066 /usr/X11R6/lib/libXtst.so.6.1 448ee000-448ef000 rw-p 00004000 03:02 3670066 /usr/X11R6/lib/libXtst.so.6.1 4caf3000-4ccf3000 r--p 00000000 03:02 458754 /usr/lib/locale/locale-archive 4cef7000-4cf13000 r--s 00000000 03:02 3081191 /workspace/java/bundles/j2sdk1.4.2/jre/lib/ext/sunjce_provider.jar 4cf13000-4cfcf000 r--s 00000000 03:02 3081365 /workspace/java/bundles/j2sdk1.4.2/jre/lib/ext/localedata.jar 4cfcf000-4d29a000 r-xp 00000000 03:02 2606276 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libawt.so 4d29a000-4d2af000 rw-p 002ca000 03:02 2606276 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libawt.so 4d2d5000-4d328000 r-xp 00000000 03:02 2606275 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libmlib_image.so 4d328000-4d329000 rw-p 00052000 03:02 2606275 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libmlib_image.so 4d329000-4d32f000 r--s 00000000 03:02 884979 /usr/lib/gconv/gconv-modules.cache 4d32f000-4d337000 r-xp 00000000 03:02 3670044 /usr/X11R6/lib/libXcursor.so.1.0 4d337000-4d338000 rw-p 00007000 03:02 3670044 /usr/X11R6/lib/libXcursor.so.1.0 4d338000-4d33f000 r-xp 00000000 03:02 3670363 /usr/X11R6/lib/libXrender.so.1.2 4d33f000-4d340000 rw-p 00006000 03:02 3670363 /usr/X11R6/lib/libXrender.so.1.2 4d340000-4d343000 r-xp 00000000 03:02 884770 /lib/libnss_dns-2.2.93.so 4d343000-4d344000 rw-p 00002000 03:02 884770 /lib/libnss_dns-2.2.93.so 4d345000-4d393000 r-xp 00000000 03:02 3670364 /usr/X11R6/lib/libXt.so.6.0 4d393000-4d397000 rw-p 0004d000 03:02 3670364 /usr/X11R6/lib/libXt.so.6.0 4d397000-4d3a4000 r-xp 00000000 03:02 3670352 /usr/X11R6/lib/libXext.so.6.4 4d3a4000-4d3a5000 rw-p 0000c000 03:02 3670352 /usr/X11R6/lib/libXext.so.6.4 4d3a5000-4d480000 r-xp 00000000 03:02 3670348 /usr/X11R6/lib/libX11.so.6.2 4d480000-4d483000 rw-p 000db000 03:02 3670348 /usr/X11R6/lib/libX11.so.6.2 4d483000-4d48b000 r-xp 00000000 03:02 3670048 /usr/X11R6/lib/libSM.so.6.0 4d48b000-4d48c000 rw-p 00007000 03:02 3670048 /usr/X11R6/lib/libSM.so.6.0 4d48c000-4d4a0000 r-xp 00000000 03:02 3670032 /usr/X11R6/lib/libICE.so.6.3 4d4a0000-4d4a1000 rw-p 00013000 03:02 3670032 /usr/X11R6/lib/libICE.so.6.3 4d4a3000-4d4b5000 r-xp 00000000 03:02 2606288 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libjavaplugin_jni.so 4d4b5000-4d4b7000 rw-p 00011000 03:02 2606288 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libjavaplugin_jni.so 4d4cb000-4d585000 r-xp 00000000 03:02 2606278 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libfontmanager.so 4d585000-4d59f000 rw-p 000b9000 03:02 2606278 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libfontmanager.so 4d5a0000-4d5bc000 r-xp 00000000 03:02 295782 /usr/X11R6/lib/X11/locale/lib/common/ximcp.so.2 4d5bc000-4d5be000 rw-p 0001b000 03:02 295782 /usr/X11R6/lib/X11/locale/lib/common/ximcp.so.2 4d8c4000-4d8d4000 r-xp 00000000 03:02 2606270 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libnet.so 4d8d4000-4d8d5000 rw-p 0000f000 03:02 2606270 /workspace/java/bundles/j2sdk1.4.2/jre/lib/i386/libnet.so 4d8f1000-4d8fc000 r-xp 00000000 03:02 884781 /lib/libnss_nisplus-2.2.93.so 4d8fc000-4d8fd000 rw-p 0000a000 03:02 884781 /lib/libnss_nisplus-2.2.93.so 4d8fd000-4d90c000 r-xp 00000000 03:02 884785 /lib/libresolv-2.2.93.so 4d90c000-4d90d000 rw-p 0000e000 03:02 884785 /lib/libresolv-2.2.93.so Heap at VM Abort: Heap def new generation total 576K, used 386K [0x448f0000, 0x44990000, 0x44dd0000) eden space 512K, 75% used [0x448f0000, 0x44950910, 0x44970000) from space 64K, 0% used [0x44970000, 0x44970048, 0x44980000) to space 64K, 0% used [0x44980000, 0x44980000, 0x44990000) tenured generation total 1784K, used 1578K [0x44dd0000, 0x44f8e000, 0x488f0000) the space 1784K, 88% used [0x44dd0000, 0x44f5a8f8, 0x44f5aa00, 0x44f8e000) compacting perm gen total 4096K, used 2993K [0x488f0000, 0x48cf0000, 0x4c8f0000) the space 4096K, 73% used [0x488f0000, 0x48bdc420, 0x48bdc600, 0x48cf0000) Local Time = Wed Feb 19 17:55:33 2003 Elapsed Time = 5 # # HotSpot Virtual Machine Error : 11 # Error ID : 4F530E43505002EF # Please report this error at # http://java.sun.com/cgi-bin/bugreport.cgi # # Java VM: Java HotSpot(TM) Client VM (1.4.2-beta-b16 mixed mode) # # An error report file has been saved as hs_err_pid2105.log. # Please refer to the file for further information. # INTERNAL ERROR on Browser End: Plugin instance index out of bounds -65 System error?:: Resource temporarily unavailable
While joshua is looking at fixing this on the java plugin side, we should try to fix this on our side as well. rogerl, is there a chokepoint in Liveconnect where we can limit the buffers we pass to Java? What should the limit be?
*** Bug 175358 has been marked as a duplicate of this bug. ***
'jsj_ConvertJSStringToJavaString()', in jsj_convert.c processes all outgoing strings and could catch ones that are 'too big' and simply fail. But, as Nisheeth already asked, what's too big?
Thanks for coding this up, Roger! I'll apply the patch to my tree and see what happens on Linux. If we can avoid the buffer overflow error message with a 9999 character string, then 10,000 seems like a reasonable max string size to me. Any objections? Even with the patch on Mozilla's side, the JVM will continue to corrupt memory on this test case, but, at least it will do so in unpredictable ways that can't be taken advantage of by a hacker. The right way to fix this bug is whatever fix Joshua comes up with on the JVM side. Joshua, any progress on fixing this bug?
It is good idea to check string length before Convert it to Java String. I am surprise that the max length of java.lang.String is 8388608, but JPI/Mozilla also crash even though we limit the string length to be 10000. (I have tested it)
I got the reason on Java Plugin side. I am sorry that I am not the member of Sun 's JPI group, so I cc to Xiaobin who is in JPI group to fix this bug.
Even though java_class_name_jstr != null, maybe it also be invalid value because of Exception happened on JVM side. so check if exception happen or not. The long string is convert successfully even though it's length is 130000. so the key is not to limit length of string. Please review/super-review this patch. Thanks a lot!
liveconnect/JPI -> me
Sorry, liveconnect/JPI -> me again
Assignee: nisheeth → joshua.xia
Status: ASSIGNED → NEW
There are many calls to the JNI throughout the LiveConnect that check for NULL return, but don't check for an exception. Do they all need to be changed now?
In this case, the java_class_name_jstr that got from JPI is invalid even though it is not null because Eception has happened. Checking all exception will reduce performance.
Status: NEW → ASSIGNED
Joshua, we want to get this in for Mozilla 1.4 - who should review it?
I'm happy with the patch, but I'd like to understand better exactly when the JPI requires us to test for exceptions, and when to just rely on the return value. Is it only for the case of getting the class name string?
I think liveconnect module peer/owner should review/super-review this patch. Roger, do you think this patch work? Strictly, to avoid this kind of bug, all JNI methods should check Exception that happened on JAVA side, but it will reduce performance. This bug can also be fixed on JPI side, but mozilla should be stronger even though JPI cheat Mozilla
Comment on attachment 115737 [details] [diff] [review] Check Exception after Get java_class_name_jstr r=rogerl on this patch; though I'd like this bug to stay open; or open another one in order to address the issue of adding exception checks after all JNI calls.
Attachment #115737 - Flags: review?(rogerl) → review+
Comment on attachment 115737 [details] [diff] [review] Check Exception after Get java_class_name_jstr sr=beard
Attachment #115737 - Flags: superreview?(beard) → superreview+
Comment on attachment 115737 [details] [diff] [review] Check Exception after Get java_class_name_jstr Requesting driver approval for checking in a security fix for a buffer overrun that happens while making Liveconnect calls. The fix is a simple two line check for an error condition and is low risk. It has been reviewed by rogerl and sr'd by beard.
Attachment #115737 - Flags: approval1.4a?
Comment on attachment 115737 [details] [diff] [review] Check Exception after Get java_class_name_jstr Please comment the unreliability of the return value in this case with a big XXX comment. I can't believe that this is the desired behavior of the underlying JVM's API. It's not sane to require a bunch of function calls after failing to return a perfectly unambiguous error code (a null pointer). Was the underlying API always like this? What char * is returned in the exception case? /be
I'm convinced this is a JNI bug, whether implementation regression or initial design flaw. Don't return a dangling pointer on error/exception; return null. Period, end of story! Joshua, what can be done to fix the JNI implementation or the underlying JVM (where ever the bug may be)? /be
It is strange that the windoze jvm is not affected by this.
JPI side return a random jstr value to js/liveconnect after a Exception happened, but the memeber in JPI team said that it is wrong way to tell jstr is invalid or valid by using jstr != 0 because sometime return value = 0 is also valid. and they suggest to check exception.
Joshua: under what circumstances is 0 (null) a valid return from the JPI function in question? I don't believe null is a valid Java string value. /be
And if null is a valid, non-error, non-exceptional return value, then the JPI should still return null on error, so that we have to check for an exception only in that case (which is rare). In no case should a random non-null pointer be returned after an exception. /be
In fact, js/liveconnect side can only believe the value that return from JPI just when there is no Exception happened on JPI/JVM side. yes, you are right, Brendan, I suggested JPI don't return random value to js/liveconnect (just init the return value to be 0) when Exception happened.
Comment on attachment 115737 [details] [diff] [review] Check Exception after Get java_class_name_jstr a=asa (on behalf of drivers) for checkin to 1.4a
Attachment #115737 - Flags: approval1.4a? → approval1.4a+
This bug is a Java Plug-in bug. Java Plug-in consumes the exception so that browser continues the call and it finally cause the crash. It has been fixed in JRE 1.4.2
Joshua, can you land this ASAP and with an XXX comment citing this bug number and asking for the patch to be undone once we know we're using a fixed JVM? Thanks.
One more request: can we make the patch's new code be #ifdef XP_UNIX only? The Java plugin bug doesn't bite on Windows, apparently. In order to be able, later, and safely, to remove this XXX comment and the extra exception-testing code, we need to know that we have a fixed Java plugin. How can we tell that, programmatically? Is there an API that LiveConnect could use to query the major and minor version, and require a sufficiently high number in either or both? /be
I think xiaobin confused this bug with another bug that our QA reported. That bug 's reason is because JPI consumes Exception and this bug is because of Mozilla has not checked Exception from JPI side. So I think the exception checking still need. is that following patch OK? + /* For fixing bugzilla #183092 + * Still need to check Exception even though java_class_name_jstr != null */ +#ifdef XP_UNIX + + if ((*jEnv)->ExceptionOccurred(jEnv)) + goto error; + +#endif if it is OK, I will make patch ASAP and checkin. Thanks!
Joshua, please check a patch like that one in today, as soon as possible (it would be nice to put the comment inside the #ifdef, btw). Thanks. /be
You means checkin, right?
Yes, please check in today. /be
The following patch is checked in, Thanks! Index: js/src/liveconnect/jsj_class.c =================================================================== RCS file: /cvsroot/mozilla/js/src/liveconnect/jsj_class.c,v retrieving revision 1.20 diff -u -r1.20 jsj_class.c --- js/src/liveconnect/jsj_class.c 19 Mar 2002 04:28:46 -0000 1.20 +++ js/src/liveconnect/jsj_class.c 1 Apr 2003 02:50:30 -0000 @@ -80,6 +80,14 @@ if (!java_class_name_jstr) goto error; + /* Fix bugzilla #183092 + * It is necessary to check Exception from JPI + * even though java_class_name_jstr != null */ +#ifdef XP_UNIX + if ((*jEnv)->ExceptionOccurred(jEnv)) + goto error; +#endif + /* Convert to UTF8 encoding and copy */ java_class_name = jsj_DupJavaStringUTF(cx, jEnv, java_class_name_jstr);
checked in and mark fixed.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Bugs published on the Known-vulnerabilities page long ago, removing confidential flag.
You need to log in before you can comment on or make changes to this bug.