-
Notifications
You must be signed in to change notification settings - Fork 311
Support multiple recipient_delimiters #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@sirainen Would you mind taking a look at this patch? I'm glad to revise it and address feedback if needed. |
The only minor thing is that for coding style consistency just use "char delim" instead of "const char delim" in the parameters. Anyway this changes existing behavior (it was already possible to use delimiter-string), so it's a v2.3 change. |
The recipient_delimiter is treated as multiple one-character delimiters rather than one multi-character delimiter if more than one character is supplied. The address is split on the first character in recipient_delimiter found.
@sirainen Thanks for taking a look! Pardon my ignorance about the dovecot development cycle, but what do I need to do to ensure this makes it into v2.3 (and that the accompanying pigeonhole patch goes in at the same time)? Also, I think I've addressed your 'const' comment, but let me know if you meant something different from the current patch. |
@@ -308,7 +308,7 @@ address_add_detail(struct client *client, const char *username, | |||
} | |||
|
|||
static bool client_proxy_rcpt(struct client *client, const char *address, | |||
const char *username, const char *detail, | |||
const char *username, const char *detail, char delim, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be const char delim
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This originally was const, but I changed it after the earlier comment by @sirainen. I'm willing to change it back if that is the consensus, or if I misinterpreted his earlier comment.
Ah my bad. |
We are currently reviewing your patch internally. |
Closing since this has been committed as 972c917 |
Are there any plans for this patch to be included in a future release version? |
As you can see it has been merged, outside github though. |
Thank for your fast reaction time. I understood that this patch was merged as 972c917 into master, but 972c917 is missing from the release branches, e.g. version 2.2.28: https://github.com/dovecot/core/commits/2.2.28/src/lmtp/commands.c I would assume this commit (972c917) to be between aeea3db and 6de95f6. |
it will probably be in 2.3 |
OK thanks! |
Some news about this commit? and when you think to release 2.3? Thank you |
It's already in v2.3 (which is currently master). |
@cmouse I wanted to tell, a real tarball ;-) yep, I have seen the commit in master, thank you. in fact, I use the tvial/docker-mailserver docker image, and I need the support of a multi recipient delimiter because sometimes some sites do not support the '+' character :/ and the alternative is '-'. Issue, I already have some emails with the '+' character. Do you have a milestone ? Thank you |
This fixes a race condition where the http_client_host_shared_idle_timeout() function would get called with an already freed hshared argument. Specifically, the situation arises from the hshared idle timeout calling http_client_host_shared_free(), which removes the timeout and then proceeds to free the client queue. The client queue freeing code indirectly calls http_client_host_shared_check_idle(), which notices that there is no idle timeout and allocates one. The backtrace at the point of this new timeout allocation: frame #3: 0x00007f0c775897f0 libdovecot.so.0`timeout_add_to(...) ioloop.c:280 frame #4: 0x00007f0c7751a45f libdovecot.so.0`http_client_host_shared_check_idle(hshared=<unavailable>) at http-client-host.c:69 frame #5: 0x00007f0c7750de89 libdovecot.so.0`http_client_request_error(_req=<unavailable>, status=9000, error="") at http-client-request.c:1525 frame #6: 0x00007f0c77517f38 libdovecot.so.0`http_client_queue_fail_full(queue=0x000055e13cff0e10, status=9000, error="", all=<unavailable>) at http-client-queue.c:183 frame #7: 0x00007f0c77518baa libdovecot.so.0`http_client_queue_free(queue=0x000055e13cff0e10) at http-client-queue.c:141 frame #8: 0x00007f0c7751a8bc libdovecot.so.0`http_client_host_free_shared(_host=<unavailable>) at http-client-host.c:391 frame #9: 0x00007f0c7751ab4c libdovecot.so.0`http_client_host_shared_free(_hshared=0x00007ffdac109e48) at http-client-host.c:294 frame #10: 0x00007f0c7751ace8 libdovecot.so.0`http_client_host_shared_idle_timeout(hshared=<unavailable>) at http-client-host.c:40 frame #11: 0x00007f0c7758a1a4 libdovecot.so.0`io_loop_handle_timeouts at ioloop.c:682 frame #12: 0x00007f0c7758a089 libdovecot.so.0`io_loop_handle_timeouts(ioloop=0x000055e13cfc8d80) at ioloop.c:696 frame #13: 0x00007f0c7758befc libdovecot.so.0`io_loop_handler_run_internal(ioloop=0x000055e13cfc8d80) at ioloop-select.c:126 frame #14: 0x00007f0c7758a56d libdovecot.so.0`io_loop_handler_run(ioloop=<unavailable>) at ioloop.c:767 frame #15: 0x00007f0c7758a798 libdovecot.so.0`io_loop_run(ioloop=0x000055e13cfc8d80) at ioloop.c:740 frame #16: 0x00007f0c774f61eb libdovecot.so.0`master_service_run(service=0x000055e13cfc8c10, callback=<unavailable>) at master-service.c:782 frame #17: 0x000055e13b48e3a5 stats`main(argc=<unavailable>, argv=<unavailable>) at main.c:99 frame #18: 0x00007f0c771092e1 libc.so.6`__libc_start_main + 241 frame #19: 0x000055e13b48e41a stats`_start + 42
This fixes a race condition where the http_client_host_shared_idle_timeout() function would get called with an already freed hshared argument. Specifically, the situation arises from the hshared idle timeout calling http_client_host_shared_free(), which removes the timeout and then proceeds to free the client queue. The client queue freeing code indirectly calls http_client_host_shared_check_idle(), which notices that there is no idle timeout and allocates one. The backtrace at the point of this new timeout allocation: frame #3: 0x00007f0c775897f0 libdovecot.so.0`timeout_add_to(...) ioloop.c:280 frame #4: 0x00007f0c7751a45f libdovecot.so.0`http_client_host_shared_check_idle(hshared=<unavailable>) at http-client-host.c:69 frame #5: 0x00007f0c7750de89 libdovecot.so.0`http_client_request_error(_req=<unavailable>, status=9000, error="") at http-client-request.c:1525 frame #6: 0x00007f0c77517f38 libdovecot.so.0`http_client_queue_fail_full(queue=0x000055e13cff0e10, status=9000, error="", all=<unavailable>) at http-client-queue.c:183 frame #7: 0x00007f0c77518baa libdovecot.so.0`http_client_queue_free(queue=0x000055e13cff0e10) at http-client-queue.c:141 frame #8: 0x00007f0c7751a8bc libdovecot.so.0`http_client_host_free_shared(_host=<unavailable>) at http-client-host.c:391 frame #9: 0x00007f0c7751ab4c libdovecot.so.0`http_client_host_shared_free(_hshared=0x00007ffdac109e48) at http-client-host.c:294 frame #10: 0x00007f0c7751ace8 libdovecot.so.0`http_client_host_shared_idle_timeout(hshared=<unavailable>) at http-client-host.c:40 frame #11: 0x00007f0c7758a1a4 libdovecot.so.0`io_loop_handle_timeouts at ioloop.c:682 frame #12: 0x00007f0c7758a089 libdovecot.so.0`io_loop_handle_timeouts(ioloop=0x000055e13cfc8d80) at ioloop.c:696 frame #13: 0x00007f0c7758befc libdovecot.so.0`io_loop_handler_run_internal(ioloop=0x000055e13cfc8d80) at ioloop-select.c:126 frame #14: 0x00007f0c7758a56d libdovecot.so.0`io_loop_handler_run(ioloop=<unavailable>) at ioloop.c:767 frame #15: 0x00007f0c7758a798 libdovecot.so.0`io_loop_run(ioloop=0x000055e13cfc8d80) at ioloop.c:740 frame #16: 0x00007f0c774f61eb libdovecot.so.0`master_service_run(service=0x000055e13cfc8c10, callback=<unavailable>) at master-service.c:782 frame #17: 0x000055e13b48e3a5 stats`main(argc=<unavailable>, argv=<unavailable>) at main.c:99 frame #18: 0x00007f0c771092e1 libc.so.6`__libc_start_main + 241 frame #19: 0x000055e13b48e41a stats`_start + 42
This fixes a race condition where the http_client_host_shared_idle_timeout() function would get called with an already freed hshared argument. Specifically, the situation arises from the hshared idle timeout calling http_client_host_shared_free(), which removes the timeout and then proceeds to free the client queue. The client queue freeing code indirectly calls http_client_host_shared_check_idle(), which notices that there is no idle timeout and allocates one. The backtrace at the point of this new timeout allocation: frame #3: 0x00007f0c775897f0 libdovecot.so.0`timeout_add_to(...) ioloop.c:280 frame #4: 0x00007f0c7751a45f libdovecot.so.0`http_client_host_shared_check_idle(hshared=<unavailable>) at http-client-host.c:69 frame #5: 0x00007f0c7750de89 libdovecot.so.0`http_client_request_error(_req=<unavailable>, status=9000, error="") at http-client-request.c:1525 frame #6: 0x00007f0c77517f38 libdovecot.so.0`http_client_queue_fail_full(queue=0x000055e13cff0e10, status=9000, error="", all=<unavailable>) at http-client-queue.c:183 frame #7: 0x00007f0c77518baa libdovecot.so.0`http_client_queue_free(queue=0x000055e13cff0e10) at http-client-queue.c:141 frame #8: 0x00007f0c7751a8bc libdovecot.so.0`http_client_host_free_shared(_host=<unavailable>) at http-client-host.c:391 frame #9: 0x00007f0c7751ab4c libdovecot.so.0`http_client_host_shared_free(_hshared=0x00007ffdac109e48) at http-client-host.c:294 frame #10: 0x00007f0c7751ace8 libdovecot.so.0`http_client_host_shared_idle_timeout(hshared=<unavailable>) at http-client-host.c:40 frame #11: 0x00007f0c7758a1a4 libdovecot.so.0`io_loop_handle_timeouts at ioloop.c:682 frame #12: 0x00007f0c7758a089 libdovecot.so.0`io_loop_handle_timeouts(ioloop=0x000055e13cfc8d80) at ioloop.c:696 frame #13: 0x00007f0c7758befc libdovecot.so.0`io_loop_handler_run_internal(ioloop=0x000055e13cfc8d80) at ioloop-select.c:126 frame #14: 0x00007f0c7758a56d libdovecot.so.0`io_loop_handler_run(ioloop=<unavailable>) at ioloop.c:767 frame #15: 0x00007f0c7758a798 libdovecot.so.0`io_loop_run(ioloop=0x000055e13cfc8d80) at ioloop.c:740 frame #16: 0x00007f0c774f61eb libdovecot.so.0`master_service_run(service=0x000055e13cfc8c10, callback=<unavailable>) at master-service.c:782 frame #17: 0x000055e13b48e3a5 stats`main(argc=<unavailable>, argv=<unavailable>) at main.c:99 frame #18: 0x00007f0c771092e1 libc.so.6`__libc_start_main + 241 frame #19: 0x000055e13b48e41a stats`_start + 42
The input string `=0A=0D ` contains on only four symbols: \n, \r, and two whitespaces. Reported by address sanitizer: istream qp decoder 1 ................................................. : ok istream qp decoder 2 ................................................. : ok istream qp decoder 3 ................................................. : ok istream qp decoder 4 ................................................. : ok istream qp decoder 5 ................................................. : ok istream qp decoder 6 ................................................. : ok istream qp decoder 7 ................................................. : ok istream qp decoder 8 ................................................. : ok istream qp decoder 9 ................................................. : ok ================================================================= ==135439==ERROR: AddressSanitizer: global-buffer-overflow on address 0x561b9fbf5489 at pc 0x561b9fb2601d bp 0x7fffdea7cba0 sp 0x7fffdea7cb98 READ of size 1 at 0x561b9fbf5489 thread T0 #0 0x561b9fb2601c in get_encoding_size_diff src/lib-mail/test-istream-qp-decoder.c:76 dovecot#1 0x561b9fb2601c in decode_test src/lib-mail/test-istream-qp-decoder.c:160 dovecot#2 0x561b9fb2601c in test_istream_qp_decoder src/lib-mail/test-istream-qp-decoder.c:183 dovecot#3 0x561b9fb1c97a in test_run_funcs ../lib-test/test-common.c:346 dovecot#4 0x561b9fb1cb1a in test_run ../lib-test/test-common.c:417 dovecot#5 0x561b9fb1cb35 in main src/lib-mail/test-istream-qp-decoder.c:196 dovecot#6 0x7f8d59240c89 (/lib/x86_64-linux-gnu/libc.so.6+0x29c89) (BuildId: 652dfccae16d17796a09de192ed332fd65dc9abb) dovecot#7 0x7f8d59240d44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29d44) (BuildId: 652dfccae16d17796a09de192ed332fd65dc9abb) dovecot#8 0x561b9fb05bc0 in _start (/build/dovecot-2.3.21+dfsg1/src/lib-mail/test-istream-qp-decoder+0x184bc0) (BuildId: c3eb31a408186312e51514f84e299274e772e6ae) 0x561b9fbf5489 is located 55 bytes before global variable '*.LC247' defined in './test-istream-qp-decoder.ltrans0.ltrans' (0x561b9fbf54c0) of size 3 '*.LC247' is ascii string ' ' 0x561b9fbf5489 is located 0 bytes after global variable '*.LC246' defined in './test-istream-qp-decoder.ltrans0.ltrans' (0x561b9fbf5480) of size 9 '*.LC246' is ascii string '=0A=0D ' SUMMARY: AddressSanitizer: global-buffer-overflow src/lib-mail/test-istream-qp-decoder.c:76 in get_encoding_size_diff Shadow bytes around the buggy address: 0x561b9fbf5200: f9 f9 f9 f9 00 00 03 f9 f9 f9 f9 f9 00 00 00 01 0x561b9fbf5280: f9 f9 f9 f9 00 00 03 f9 f9 f9 f9 f9 00 00 06 f9 0x561b9fbf5300: f9 f9 f9 f9 00 02 f9 f9 f9 f9 f9 f9 00 00 00 02 0x561b9fbf5380: f9 f9 f9 f9 00 00 00 00 03 f9 f9 f9 f9 f9 f9 f9 0x561b9fbf5400: 05 f9 f9 f9 f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 =>0x561b9fbf5480: 00[01]f9 f9 f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 0x561b9fbf5500: 00 f9 f9 f9 f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 0x561b9fbf5580: 05 f9 f9 f9 f9 f9 f9 f9 07 f9 f9 f9 f9 f9 f9 f9 0x561b9fbf5600: 00 01 f9 f9 f9 f9 f9 f9 00 00 03 f9 f9 f9 f9 f9 0x561b9fbf5680: 07 f9 f9 f9 f9 f9 f9 f9 00 00 00 04 f9 f9 f9 f9 0x561b9fbf5700: 00 00 00 f9 f9 f9 f9 f9 07 f9 f9 f9 f9 f9 f9 f9 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==135439==ABORTING Fixes: e3b45a1 ("lib-mail: Extend quoted-printable decoding tests")
The recipient_delimiter is treated as multiple one-character delimiters
rather than one multi-character delimiter if more than one character is
supplied. The address is split on the first character in
recipient_delimiter found.