redsolution / xabber-android

Open-source XMPP client for Android
http://xabber.com
Other
1.82k stars 815 forks source link

Crash on incoming message "?OTR" #329

Open grigoryfedorov opened 9 years ago

grigoryfedorov commented 9 years ago

01-13 17:11:52.567 23597-23597/com.xabber.androiddev E/AndroidRuntime﹕ FATAL EXCEPTION: main Process: com.xabber.androiddev, PID: 23597 java.lang.StringIndexOutOfBoundsException: length=4; index=4 at java.lang.String.indexAndLength(String.java:500) at java.lang.String.charAt(String.java:494) at net.java.otr4j.io.SerializationUtils.toMessage(SerializationUtils.java:263) at net.java.otr4j.session.SessionImpl.transformReceiving(SessionImpl.java:302) at com.xabber.android.data.extension.otr.OTRManager.transformReceiving(OTRManager.java:450) at com.xabber.android.data.message.RegularChat.onPacket(RegularChat.java:145) at com.xabber.android.data.message.MessageManager.onPacket(MessageManager.java:450) at com.xabber.android.data.connection.ConnectionManager.processPacket(ConnectionManager.java:275) at com.xabber.android.data.connection.ConnectionThread$18.run(ConnectionThread.java:598) at android.os.Handler.handleCallback(Handler.java:739) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:135) at android.app.ActivityThread.main(ActivityThread.java:5221) at java.lang.reflect.Method.invoke(Native Method) at java.lang.reflect.Method.invoke(Method.java:372) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694)

aileronajay commented 9 years ago

Is the error coming from

char contentType = s.charAt(idxHead + SerializationConstants.HEAD.length());

in the method net.java.otr4j.io.SerializationUtils.toMessage? Is it due to incorrectly constructed message header or is the code assuming an incorrect header length?

aileronajay commented 9 years ago

I read the bug title and i see that incoming message is "?OTR", in this scenario, the above code will fail. I wonder if this should be handled by xabber code or if it be an enhancement for OTR code. Till the time this is fixed in OTR, we can pre validate before delegating to OTR whether the string is only "?OTR"

aileronajay commented 9 years ago

by reviewing the protocol details over here ,https://otr.cypherpunks.ca/Protocol-v2-3.1.0.html, i was wondering if having only "?OTR" as a message is a valid use case. We need atleast one more character after it to get the version