Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#411 closed defect (fixed)

misallocation with ngx_mail_smtp_module at STARTTLS

Reported by: markus-linnala.myopenid.com Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.3.x
Keywords: Cc:
uname -a:
nginx -V: nginx version: nginx/1.5.5

Description

I modified ​http://mdounin.ru/hg/nginx-tests to use valgrind.

Test was mail_imap.t

==10647== Invalid write of size 1
==10647== at 0x4B1493: ngx_mail_smtp_merge_srv_conf (ngx_mail_smtp_module.c:280)
==10647== by 0x4AB363: ngx_mail_block (ngx_mail.c:209)
==10647== by 0x4303BE: ngx_conf_parse (ngx_conf_file.c:391)
==10647== by 0x42DF03: ngx_init_cycle (ngx_cycle.c:265)
==10647== by 0x4206A9: main (nginx.c:333)
==10647== Address 0x550fb84 is 0 bytes after a block of size 68 alloc'd
==10647== at 0x4A074CD: malloc (vg_replace_malloc.c:236)
==10647== by 0x43B251: ngx_alloc (ngx_alloc.c:22)
==10647== by 0x421B0D: ngx_malloc (ngx_palloc.c:119)
==10647== by 0x421B65: ngx_pnalloc (ngx_palloc.c:147)
==10647== by 0x4B1447: ngx_mail_smtp_merge_srv_conf (ngx_mail_smtp_module.c:269)
==10647== by 0x4AB363: ngx_mail_block (ngx_mail.c:209)
==10647== by 0x4303BE: ngx_conf_parse (ngx_conf_file.c:391)
==10647== by 0x42DF03: ngx_init_cycle (ngx_cycle.c:265)
==10647== by 0x4206A9: main (nginx.c:333)
==10647==

Clearly memory for second CR + LF is not allocated.

Attachments (1)

valgrind-mail-misalloc.patch (2.2 KB ) - added by markus-linnala.myopenid.com 7 years ago.

Download all attachments as: .zip

Change History (7)

by markus-linnala.myopenid.com, 7 years ago

comment:1 by Maxim Dounin, 7 years ago

Status: newaccepted

It looks like the proper fix would be to just remove extra CRLF. Care to provide a patch following http://nginx.org/en/docs/contributing_changes.html?

comment:2 by markus-linnala.myopenid.com, 7 years ago

I wondered about extra CRLF. But after some delibration I choose not to drop extra CRLF. Currently I can not test change as mail proxy as a proper configuration I do not want to change what nginx sends. This extra CRLF has been from the start of mail implementation as far as I was able to see.

I have submitted patch via nginx-devel mailing list as per documentation.

comment:3 by Maxim Dounin <mdounin@…>, 7 years ago

In baa705805138dfacdb85dc167db25627f81ceca3/nginx:

Mail: fixed overrun of allocated memory (ticket #411).

Reported by Markus Linnala.

comment:4 by Maxim Dounin, 7 years ago

Resolution: fixed
Status: acceptedclosed

Fix committed, thnx.

comment:5 by Maxim Dounin <mdounin@…>, 7 years ago

In 66db21c0ac5526ec239921aac9df3d92e33f1b5f/nginx:

Mail: fixed overrun of allocated memory (ticket #411).

Reported by Markus Linnala.

comment:6 by Maxim Dounin, 3 years ago

sensitive: 10
Note: See TracTickets for help on using tickets.