Opened 7 months ago

Closed 7 months ago

#1751 closed defect (fixed)

nginx memory leak in ngx_event_openssl.c with patch

Reported by: NMorozxov@… 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 NMorozxov@… 7 months ago.

Download all attachments as: .zip

Change History (8)

Changed 7 months ago by NMorozxov@…

comment:1 Changed 7 months ago by pluknet

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 Changed 7 months ago by mdounin

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 Changed 7 months ago by NMorozxov@…

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

comment:4 Changed 7 months ago by NMorozxov@…

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 Changed 7 months ago by mdounin

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 Changed 7 months ago by NMorozxov@…

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

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

  • Owner set to Nikolay Morozov <n.morozov@…>
  • Resolution set to fixed
  • Status changed from new to closed

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.