POP3 LIST request never terminates if there are zero messages. by bnoordhuis · Pull Request #10 · curl/curl

bnoordhuis · 2011-03-17T20:57:08Z

I tried to write a test case for this but gave up after two rather frustrating hours with the test framework.

To reproduce, do a request to an empty mailbox:

$ src/curl -v pop3://testuser:testuser@localhost/
* About to connect() to localhost port 110 (#0)
*   Trying ::1... Connection refused
*   Trying 127.0.0.1... connected
* Connected to localhost (127.0.0.1) port 110 (#0)
* POP3 0x24bfbb8 state change from STOP to SERVERGREET
< +OK Dovecot ready.
> USER testuser
* POP3 0x24bfbb8 state change from SERVERGREET to USER
< +OK
> PASS testuser
* POP3 0x24bfbb8 state change from USER to PASS
< +OK Logged in.
* POP3 0x24bfbb8 state change from PASS to STOP
* DO phase starts
> LIST 
* POP3 0x24bfbb8 state change from STOP to LIST
< +OK 0 messages:
.
* POP3 0x24bfbb8 state change from LIST to STOP
* DO phase is complete

That last line is where curl hangs. The reply from the server ends with .\r\n but the response parser expects to see \r\n.\r\n.

Curl_pop3_write() expects the response to end in "\r\n.\r\n" but an empty LIST reply consists of the string ".\r\n" only.

bnoordhuis · 2011-03-17T21:04:16Z

By the way, this snippet in pop3_list() is essentially dead code (but correct dead code now): pop3c->mailbox is always empty here because pop3_perform() issues a RETR command if it isn't.

dfandrich · 2011-03-18T00:07:07Z

On Thu, Mar 17, 2011 at 01:57:09PM -0700, bnoordhuis wrote:

I tried to write a test case for this but gave up after two rather frustrating hours with the test framework.

To reproduce, do a request to an empty mailbox:

$ src/curl -v pop3://testuser:testuser@localhost/

About to connect() to localhost port 110 (#0)

Trying ::1... Connection refused

Trying 127.0.0.1... connected

Connected to localhost (127.0.0.1) port 110 (#0)

POP3 0x24bfbb8 state change from STOP to SERVERGREET
< +OK Dovecot ready.

USER testuser

POP3 0x24bfbb8 state change from SERVERGREET to USER
< +OK

PASS testuser

POP3 0x24bfbb8 state change from USER to PASS
< +OK Logged in.

POP3 0x24bfbb8 state change from PASS to STOP

DO phase starts

LIST

POP3 0x24bfbb8 state change from STOP to LIST
< +OK 0 messages:
.

POP3 0x24bfbb8 state change from LIST to STOP

DO phase is complete

That last line is where curl hangs. The reply from the server ends with .\r\n but the response parser expects to see \r\n.\r\n.

I've added test case 811 to test this scenario. Would you mind rebasing and
trying it again? I'm not sure the test for "+OK 0" in that patch is
valid. I don't see anything in the POP3 spec that says a server has to
list the number of messages in the +OK response line for a LIST command
that doesn't specify a message number, and test case 811 doesn't do so.

Dan

bnoordhuis · 2011-03-18T12:11:48Z

You're technically correct (the best kind of correct). The major POP3 implementations all try very hard to be compatible with UCB so it's probably not an issue in everyday life but still, correctness first. I won't have the time to revisit this in the next few days, though.

By the way, test 811 triggers the old behaviour so it still hangs.

dfandrich · 2011-03-18T14:34:33Z

On Fri, Mar 18, 2011 at 05:11:50AM -0700, bnoordhuis wrote:

You're technically correct (the best kind of correct). The major POP3 implementations all try very hard to be compatible with UCB so it's probably not an issue in everyday life but still, correctness first. I won't have the time to revisit this in the next few days, though.

By the way, test 811 triggers the old behaviour so it still hangs.

That's why I checked it in as disabled :-) It's there so you or someone
can use it to verify the eventual fix.

Dan

bagder · 2011-04-09T22:23:35Z

I really think this should be done differently and I want to have the discussion about it on the mailing list. Please post patches there and we can discuss it there.

bagder · 2011-04-14T20:31:32Z

Please send patches to the curl-library mailing list for discussion and public review.

In short the easy handle needs to be disconnected from its connection at this point since the connection still is serving other easy handles. In our app we can reliably reproduce a crash in our http2 stress test that is fixed by this change. I can't easily reproduce the same test in a small example. This is the gdb/asan output: ==11785==ERROR: AddressSanitizer: heap-use-after-free on address 0xe9f4fb80 at pc 0x09f41f19 bp 0xf27be688 sp 0xf27be67c READ of size 4 at 0xe9f4fb80 thread T13 (RESOURCE_HTTP) #0 0x9f41f18 in curl_multi_remove_handle /path/to/source/3rdparty/curl/lib/multi.c:666 0xe9f4fb80 is located 0 bytes inside of 1128-byte region [0xe9f4fb80,0xe9f4ffe8) freed by thread T13 (RESOURCE_HTTP) here: #0 0xf7b1b5c2 in __interceptor_free /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_malloc_linux.cc:45 curl#1 0x9f7862d in conn_free /path/to/source/3rdparty/curl/lib/url.c:2808 curl#2 0x9f78c6a in Curl_disconnect /path/to/source/3rdparty/curl/lib/url.c:2876 curl#3 0x9f41b09 in multi_done /path/to/source/3rdparty/curl/lib/multi.c:615 curl#4 0x9f48017 in multi_runsingle /path/to/source/3rdparty/curl/lib/multi.c:1896 curl#5 0x9f490f1 in curl_multi_perform /path/to/source/3rdparty/curl/lib/multi.c:2123 curl#6 0x9c4443c in perform /path/to/source/src/net/resourcemanager/ResourceManagerCurlThread.cpp:854 curl#7 0x9c445e0 in ... curl#8 0x9c4cf1d in ... curl#9 0xa2be6b5 in ... curl#10 0xf7aa5780 in asan_thread_start /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_interceptors.cc:226 curl#11 0xf4d3a16d in __clone (/lib/i386-linux-gnu/libc.so.6+0xe716d) previously allocated by thread T13 (RESOURCE_HTTP) here: #0 0xf7b1ba27 in __interceptor_calloc /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_malloc_linux.cc:70 curl#1 0x9f7dfa6 in allocate_conn /path/to/source/3rdparty/curl/lib/url.c:3904 curl#2 0x9f88ca0 in create_conn /path/to/source/3rdparty/curl/lib/url.c:5797 curl#3 0x9f8c928 in Curl_connect /path/to/source/3rdparty/curl/lib/url.c:6438 curl#4 0x9f45a8c in multi_runsingle /path/to/source/3rdparty/curl/lib/multi.c:1411 curl#5 0x9f490f1 in curl_multi_perform /path/to/source/3rdparty/curl/lib/multi.c:2123 curl#6 0x9c4443c in perform /path/to/source/src/net/resourcemanager/ResourceManagerCurlThread.cpp:854 curl#7 0x9c445e0 in ... curl#8 0x9c4cf1d in ... curl#9 0xa2be6b5 in ... curl#10 0xf7aa5780 in asan_thread_start /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_interceptors.cc:226 curl#11 0xf4d3a16d in __clone (/lib/i386-linux-gnu/libc.so.6+0xe716d) SUMMARY: AddressSanitizer: heap-use-after-free /path/to/source/3rdparty/curl/lib/multi.c:666 in curl_multi_remove_handle Shadow bytes around the buggy address: 0x3d3e9f20: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f50: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa 0x3d3e9f60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x3d3e9f70:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9fa0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9fb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9fc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 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 Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 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 ==11785==ABORTING Thread 14 "RESOURCE_HTTP" received signal SIGABRT, Aborted. [Switching to Thread 0xf27bfb40 (LWP 12324)] 0xf7fd8be9 in __kernel_vsyscall () (gdb) bt #0 0xf7fd8be9 in __kernel_vsyscall () curl#1 0xf4c7ee89 in __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:54 curl#2 0xf4c803e7 in __GI_abort () at abort.c:89 curl#3 0xf7b2ef2e in __sanitizer::Abort () at /opt/toolchain/src/gcc-6.2.0/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cc:122 curl#4 0xf7b262fa in __sanitizer::Die () at /opt/toolchain/src/gcc-6.2.0/libsanitizer/sanitizer_common/sanitizer_common.cc:145 curl#5 0xf7b21ab3 in __asan::ScopedInErrorReport::~ScopedInErrorReport (this=0xf27be171, __in_chrg=<optimized out>) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_report.cc:689 curl#6 0xf7b214a5 in __asan::ReportGenericError (pc=166993689, bp=4068206216, sp=4068206204, addr=3925146496, is_write=false, access_size=4, exp=0, fatal=true) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_report.cc:1074 curl#7 0xf7b21fce in __asan::__asan_report_load4 (addr=3925146496) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_rtl.cc:129 curl#8 0x09f41f19 in curl_multi_remove_handle (multi=0xf3406080, data=0xde582400) at /path/to/source3rdparty/curl/lib/multi.c:666 curl#9 0x09f6b277 in Curl_close (data=0xde582400) at /path/to/source3rdparty/curl/lib/url.c:415 curl#10 0x09f3354e in curl_easy_cleanup (data=0xde582400) at /path/to/source3rdparty/curl/lib/easy.c:860 curl#11 0x09c6de3f in ... curl#12 0x09c378c5 in ... curl#13 0x09c48133 in ... curl#14 0x09c4d092 in ... curl#15 0x0a2be6b6 in ... curl#16 0xf7aa5781 in asan_thread_start (arg=0xf2d22938) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_interceptors.cc:226 curl#17 0xf5de52b5 in start_thread (arg=0xf27bfb40) at pthread_create.c:333 curl#18 0xf4d3a16e in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:114

In short the easy handle needs to be disconnected from its connection at this point since the connection still is serving other easy handles. In our app we can reliably reproduce a crash in our http2 stress test that is fixed by this change. I can't easily reproduce the same test in a small example. This is the gdb/asan output: ==11785==ERROR: AddressSanitizer: heap-use-after-free on address 0xe9f4fb80 at pc 0x09f41f19 bp 0xf27be688 sp 0xf27be67c READ of size 4 at 0xe9f4fb80 thread T13 (RESOURCE_HTTP) #0 0x9f41f18 in curl_multi_remove_handle /path/to/source/3rdparty/curl/lib/multi.c:666 0xe9f4fb80 is located 0 bytes inside of 1128-byte region [0xe9f4fb80,0xe9f4ffe8) freed by thread T13 (RESOURCE_HTTP) here: #0 0xf7b1b5c2 in __interceptor_free /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_malloc_linux.cc:45 #1 0x9f7862d in conn_free /path/to/source/3rdparty/curl/lib/url.c:2808 #2 0x9f78c6a in Curl_disconnect /path/to/source/3rdparty/curl/lib/url.c:2876 #3 0x9f41b09 in multi_done /path/to/source/3rdparty/curl/lib/multi.c:615 #4 0x9f48017 in multi_runsingle /path/to/source/3rdparty/curl/lib/multi.c:1896 #5 0x9f490f1 in curl_multi_perform /path/to/source/3rdparty/curl/lib/multi.c:2123 #6 0x9c4443c in perform /path/to/source/src/net/resourcemanager/ResourceManagerCurlThread.cpp:854 #7 0x9c445e0 in ... #8 0x9c4cf1d in ... #9 0xa2be6b5 in ... #10 0xf7aa5780 in asan_thread_start /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_interceptors.cc:226 #11 0xf4d3a16d in __clone (/lib/i386-linux-gnu/libc.so.6+0xe716d) previously allocated by thread T13 (RESOURCE_HTTP) here: #0 0xf7b1ba27 in __interceptor_calloc /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_malloc_linux.cc:70 #1 0x9f7dfa6 in allocate_conn /path/to/source/3rdparty/curl/lib/url.c:3904 #2 0x9f88ca0 in create_conn /path/to/source/3rdparty/curl/lib/url.c:5797 #3 0x9f8c928 in Curl_connect /path/to/source/3rdparty/curl/lib/url.c:6438 #4 0x9f45a8c in multi_runsingle /path/to/source/3rdparty/curl/lib/multi.c:1411 #5 0x9f490f1 in curl_multi_perform /path/to/source/3rdparty/curl/lib/multi.c:2123 #6 0x9c4443c in perform /path/to/source/src/net/resourcemanager/ResourceManagerCurlThread.cpp:854 #7 0x9c445e0 in ... #8 0x9c4cf1d in ... #9 0xa2be6b5 in ... #10 0xf7aa5780 in asan_thread_start /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_interceptors.cc:226 #11 0xf4d3a16d in __clone (/lib/i386-linux-gnu/libc.so.6+0xe716d) SUMMARY: AddressSanitizer: heap-use-after-free /path/to/source/3rdparty/curl/lib/multi.c:666 in curl_multi_remove_handle Shadow bytes around the buggy address: 0x3d3e9f20: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f50: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa 0x3d3e9f60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x3d3e9f70:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9f90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9fa0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9fb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x3d3e9fc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 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 Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 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 ==11785==ABORTING Thread 14 "RESOURCE_HTTP" received signal SIGABRT, Aborted. [Switching to Thread 0xf27bfb40 (LWP 12324)] 0xf7fd8be9 in __kernel_vsyscall () (gdb) bt #0 0xf7fd8be9 in __kernel_vsyscall () #1 0xf4c7ee89 in __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #2 0xf4c803e7 in __GI_abort () at abort.c:89 #3 0xf7b2ef2e in __sanitizer::Abort () at /opt/toolchain/src/gcc-6.2.0/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cc:122 #4 0xf7b262fa in __sanitizer::Die () at /opt/toolchain/src/gcc-6.2.0/libsanitizer/sanitizer_common/sanitizer_common.cc:145 #5 0xf7b21ab3 in __asan::ScopedInErrorReport::~ScopedInErrorReport (this=0xf27be171, __in_chrg=<optimized out>) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_report.cc:689 #6 0xf7b214a5 in __asan::ReportGenericError (pc=166993689, bp=4068206216, sp=4068206204, addr=3925146496, is_write=false, access_size=4, exp=0, fatal=true) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_report.cc:1074 #7 0xf7b21fce in __asan::__asan_report_load4 (addr=3925146496) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_rtl.cc:129 #8 0x09f41f19 in curl_multi_remove_handle (multi=0xf3406080, data=0xde582400) at /path/to/source3rdparty/curl/lib/multi.c:666 #9 0x09f6b277 in Curl_close (data=0xde582400) at /path/to/source3rdparty/curl/lib/url.c:415 #10 0x09f3354e in curl_easy_cleanup (data=0xde582400) at /path/to/source3rdparty/curl/lib/easy.c:860 #11 0x09c6de3f in ... #12 0x09c378c5 in ... #13 0x09c48133 in ... #14 0x09c4d092 in ... #15 0x0a2be6b6 in ... #16 0xf7aa5781 in asan_thread_start (arg=0xf2d22938) at /opt/toolchain/src/gcc-6.2.0/libsanitizer/asan/asan_interceptors.cc:226 #17 0xf5de52b5 in start_thread (arg=0xf27bfb40) at pthread_create.c:333 #18 0xf4d3a16e in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:114 Fixes #1083

POP3 LIST request never terminates if there are zero messages.

2144cae

Curl_pop3_write() expects the response to end in "\r\n.\r\n" but an empty LIST reply consists of the string ".\r\n" only.

bagder closed this Apr 14, 2011

ghost mentioned this pull request Jan 17, 2016

Infinite loop at function Curl_llist_destroy() #604

Closed

arekm mentioned this pull request Jul 19, 2016

SSL_COMP_free_compression_methods freeing harms other openssl users in single binary (php in this case) #905

Closed

Andersbakken mentioned this pull request Oct 20, 2016

Fix a double-free for curl_multi_remove_handle #1083

Closed

usergoodvery mentioned this pull request Nov 9, 2016

valgrind noise... #1117

Closed

Pwera mentioned this pull request Apr 20, 2017

UNINITIALIZED READ" when using POST/GET on Windows 7 #1434

Closed

kolychen mentioned this pull request May 18, 2017

SIGBUS ERROR #1493

Closed

dhakiptk mentioned this pull request Feb 5, 2018

http2 assert drain_total and state.drain gets out of sync #1680

Closed

vince166 mentioned this pull request Apr 25, 2018

libcurl crashes when reusing easy_handle with FTPS sites #2530

Closed

Desperado17 mentioned this pull request Aug 1, 2018

AddressSanitizer finding in Curl-7.54.0 #2824

Closed

dinama mentioned this pull request Oct 15, 2018

7.61.1 crash on __write_nocancel:libpthread #3139

Closed

lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

POP3 LIST request never terminates if there are zero messages.#10

POP3 LIST request never terminates if there are zero messages.#10
bnoordhuis wants to merge 1 commit intocurl:masterfrom
bnoordhuis:pop3-LIST-bug

bnoordhuis commented Mar 17, 2011

Uh oh!

bnoordhuis commented Mar 17, 2011

Uh oh!

dfandrich commented Mar 18, 2011

Uh oh!

bnoordhuis commented Mar 18, 2011

Uh oh!

dfandrich commented Mar 18, 2011

Uh oh!

bagder commented Apr 9, 2011

Uh oh!

bagder commented Apr 14, 2011

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

bnoordhuis commented Mar 17, 2011

Uh oh!

bnoordhuis commented Mar 17, 2011

Uh oh!

dfandrich commented Mar 18, 2011

Uh oh!

bnoordhuis commented Mar 18, 2011

Uh oh!

dfandrich commented Mar 18, 2011

Uh oh!

bagder commented Apr 9, 2011

Uh oh!

bagder commented Apr 14, 2011

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants