Closed Bug 183092 Opened 21 years ago Closed 21 years ago

Buffer overrun calling Java from JS

Categories

(Core :: Security, defect)

Product:
Core
Component:
Security
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.4alpha

People

(Reporter: security-bugs, Assigned: joshua.xia)

References

Details

(Whiteboard: [sg:mustfix])

Attachments

(3 files)

perl cgi which demonstrates the problem
472 bytes, text/plain
Details
Tentative fix proposal
1.28 KB, patch
Details | Diff | Splinter Review
Check Exception after Get java_class_name_jstr
636 bytes, patch
rogerl
: review+
beard
: superreview+
asa
: approval1.4a+
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
to nisheeth
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.
Whiteboard: [sg:mustfix]
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...
Target Milestone: mozilla1.3beta → mozilla1.3final
Nisheeth, not seeing the alert and getting java security exception is normal
behavior and is not a bug.
Depends on: 192657
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...
No longer depends on: 192657
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?
Comments?
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?
Target Milestone: mozilla1.3final → mozilla1.4alpha
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
Attachment #115737 - Flags: review?(rogerl)
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+
Attachment #115737 - Flags: superreview?(beard)
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?
Flags: blocking1.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: 21 years ago
Resolution: --- → FIXED
Bugs published on the Known-vulnerabilities page long ago, removing confidential
flag.
Group: security
You need to log in before you can comment on or make changes to this bug.