cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] openssl: Fix uninitialized variable use in NPN callback

From: Fabian Frank <fabian.frank.de_at_gmail.com>
Date: Wed, 21 May 2014 10:01:39 -0700

On May 21, 2014, at 7:55 AM, Tatsuhiro Tsujikawa <tatsuhiro.t_at_gmail.com> wrote:

> OpenSSL passes out and outlen variable uninitialized to
> select_next_proto_cb callback function. If the callback function
> returns SSL_TLSEXT_ERR_OK, the caller assumes the callback filled
> values in out and outlen and processes as such. Previously, if there
> is no overlap in protocol lists, curl code does not fill any values in
> these variables and returns SSL_TLSEXT_ERR_OK, which means we are
> triggering undefined behavior. valgrind warns this.

In line 1430 we completely delegate this functionality to nghttp2, see their implementation of the callback:
https://github.com/tatsuhiro-t/nghttp2/blob/master/lib/nghttp2_npn.c#L29

I would recommend that we fix this in nghttp2, what do you think?

> This patch fixes this issue by filling HTTP/2 protocol identifier
> nghttp2 library supports when there is no overlap. Unlike ALPN, NPN
> specification https://technotes.googlecode.com/git/nextprotoneg.html
> says that client should select first protocol it supports if there is
> no overlap.

I don’t think that it’s feasible to always fall back to HTTP2, it
means we are almost guaranteed to fail. Falling back to HTTP 1.1 makes more sense.
We should consider HTTP 1.1 our first protocol.

Regards,
Fabian

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-05-21