#1751 closed defect (fixed)

nginx memory leak in ngx_event_openssl.c with patch

Reported by: Nikolay Morozov Owned by: Nikolay Morozov <n.morozov@…>
Priority: major Milestone:
Component: nginx-core Version: 1.14.x
Keywords: Cc:
uname -a: Linux nmorozov 4.15.0-46-generic #49-Ubuntu SMP Wed Feb 6 09:33:07 UTC 2019
nginx -V: nginx version: nginx/1.14.2

Description

on some error cases X509 certificate is leaking. I have attached the patch to correct this

Attachments (1)

nginx_leak_for_upstream.diff (575 bytes ) - added by Nikolay Morozov 17 months ago.

Download all attachments as: .zip

Change History (8)

by Nikolay Morozov, 17 months ago

comment:1 by Sergey Kandaurov, 17 months ago

The patch looks good.
Please take a look at http://nginx.org/en/docs/contributing_changes.html.
It'd be best if you could use Mercurial to submit patches.

comment:2 by Maxim Dounin, 17 months ago

Could you please clarify - are you seeing these leaks in practice, or you've found this by reading the code? It looks like these errors shouldn't happen in practice, as both X509_get_issuer_name() and X509_get_subject_name() will always return valid pointers.

comment:3 by Nikolay Morozov, 17 months ago

In searching the memory leaks in production. There is a patched nginx with many external modules. I just found them analyzing code.

comment:4 by Nikolay Morozov, 17 months ago

Both X509_get_issuer_name() and X509_get_subject_name() will always return valid pointers only if client certificate is ok, but there can be a defective certificate.
I wiil use mercurial to submit changes

comment:5 by Maxim Dounin, 17 months ago

Both X509_get_issuer_name() and X509_get_subject_name() will always return valid pointers only if client certificate is ok, but there can be a defective certificate.

Sure, but these fields are mandatory, and handshake is expected to fail if such a defective certificate is provided by the client. As such, I suspect the issue is purely theoretical, and cannot lead to real memory leaks. (It needs to be fixed anyway, of course, but the question is about possible impact.)

comment:6 by Nikolay Morozov, 17 months ago

ok, I have alreday submited the chanes to ngix-devel maillist with hg email

comment:7 by Nikolay Morozov <n.morozov@…>, 17 months ago

Owner: set to Nikolay Morozov <n.morozov@…>
Resolution: fixed
Status: newclosed

In 7484:65074e13f171/nginx:

SSL: missing free calls in $ssl_client_s_dn and $ssl_client_i_dn.

If X509_get_issuer_name() or X509_get_subject_name() returned NULL,
this could lead to a certificate reference leak. It cannot happen
in practice though, since each function returns an internal pointer
to a mandatory subfield of the certificate successfully decoded by
d2i_X509() during certificate message processing (closes #1751).

Note: See TracTickets for help on using tickets.