nn_recv: does not set the Buffer.limit(), and security bug in input arg handling

[megahall]

Something similar to the following code inside of Java_org_nanomsg_NanoLibrary_nn_1recv is required:

jclass cls = env->GetObjectClass(buffer);
jmethodID limit_mid = env->GetMethodID(cls, "limit", "(I)Ljava/nio/Buffer;");
char *buf = (char*)env->GetDirectBufferAddress(buffer);
jlong capacity = env->GetDirectBufferCapacity(buffer);

// call nn_recv function here, and store how many bytes were received
ret = nn_recv(socket, cbuf + offset, length, flags);

env->CallObjectMethod(buffer, limit_mid, ret <= 0? 0 : ret);
return written;

In addition the code has a big security bug. cbuf + offset and length are used without checking whether they are actually inside of the Buffer. This bug could be used to unexpectedly modify arbitrary memory in the JVM when using nanomsg.

[megahall]

See also:

http://stackoverflow.com/questions/11329519/manipulation-of-bytebuffer-from-jni

[megahall]

Some other checks might be needed, to maintain the Java invariant rules for Buffer:

position <= limit <= capacity

[megahall]

In this case, cbuf + offset, and cbuf + offset + length need to be less than capacity, and length needs to be less than limit - position. Negative values should also not be passed for any of these. It's worth verifying what other JVM code is doing here, like the *Channel implementations, to be sure no critical security checks get missed.

[gonzus]

Hi Mathew,

Good points, all of them. Would you be willing to provide a pull request with proposed fixes?

I suggest we keep the nanomsg list copied (I just did it).

Best regards,

Gonzalo Diethelm DCV Chile

From: Matthew Hall [mailto:[email protected]] Sent: Monday, May 12, 2014 10:45 PM To: gonzus/jnano Subject: Re: [jnano] nn_recv: does not set the Buffer.limit(), and security bug in input arg handling (#4)

In this case, cbuf + offset, and cbuf + offset + length need to be less than capacity, and length needs to be less than limit - position. Negative values should also not be passed for any of these. It's worth verifying what other JVM code is doing here, like the *Channel implementations, to be sure no critical security checks get missed.

— Reply to this email directly or view it on GitHubhttps://github.com/gonzus/jnano/issues/4#issuecomment-42911813.

---

Declaración de confidencialidad: Este Mensaje esta destinado para el uso de la o las personas o entidades a quien ha sido dirigido y puede contener información reservada y confidencial que no puede ser divulgada, difundida, ni aprovechada en forma alguna. El uso no autorizado de la información contenida en este correo podrá ser sancionado de conformidad con la ley chilena. Si usted ha recibido este correo electrónico por error, le pedimos eliminarlo junto con los archivos adjuntos y avisar inmediatamente al remitente, respondiendo este mensaje.

"Before printing this e-mail think if is really necesary". Disclosure: This Message is to be used by the individual, individuals or entities that it is addressed to and may include private and confidential information that may not be disclosed, made public nor used in any way at all. Unauthorized use of the information in this electronic mail message may be subject to the penalties set forth by Chilean law. If you have received this electronic mail message in error, we ask you to destroy the message and its attached file(s) and to immediately notify the sender by answering this message.

[megahall]

Sure, I'll see what I can do. This week is crazy but I want to try to take care of it.

Sent from my mobile device.

On May 13, 2014 5:16:27 AM PDT, Gonzalo Diethelm [email protected] wrote:

Hi Mathew,

Good points, all of them. Would you be willing to provide a pull request with proposed fixes?

I suggest we keep the nanomsg list copied (I just did it).

Best regards,

Gonzalo Diethelm DCV Chile

From: Matthew Hall [mailto:[email protected]] Sent: Monday, May 12, 2014 10:45 PM To: gonzus/jnano Subject: Re: [jnano] nn_recv: does not set the Buffer.limit(), and security bug in input arg handling (#4)

In this case, cbuf + offset, and cbuf + offset + length need to be less than capacity, and length needs to be less than limit - position. Negative values should also not be passed for any of these. It's worth verifying what other JVM code is doing here, like the *Channel implementations, to be sure no critical security checks get missed.

— Reply to this email directly or view it on GitHubhttps://github.com/gonzus/jnano/issues/4#issuecomment-42911813.

---

Declaración de confidencialidad: Este Mensaje esta destinado para el uso de la o las personas o entidades a quien ha sido dirigido y puede contener información reservada y confidencial que no puede ser divulgada, difundida, ni aprovechada en forma alguna. El uso no autorizado de la información contenida en este correo podrá ser sancionado de conformidad con la ley chilena. Si usted ha recibido este correo electrónico por error, le pedimos eliminarlo junto con los archivos adjuntos y avisar inmediatamente al remitente, respondiendo este mensaje.

"Before printing this e-mail think if is really necesary". Disclosure: This Message is to be used by the individual, individuals or entities that it is addressed to and may include private and confidential information that may not be disclosed, made public nor used in any way at all. Unauthorized use of the information in this electronic mail message may be subject to the penalties set forth by Chilean law. If you have received this electronic mail message in error, we ask you to destroy the message and its attached file(s) and to immediately notify the sender by answering this message.

---

Reply to this email directly or view it on GitHub: https://github.com/gonzus/jnano/issues/4#issuecomment-42947211