Skip to content

fts-squat: fix Corrupted squat uidlist bug #5

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fts-squat: fix Corrupted squat uidlist bug #5

wants to merge 2 commits into from

Conversation

nikwrt
Copy link
Contributor

@nikwrt nikwrt commented Mar 29, 2016

Hello,

I have experienced the "Corrupted squat uidlist" bug described in these two threads:

http://dovecot.org/list/dovecot/2012-April/135162.html
http://dovecot.org/list/dovecot/2016-January/102864.html

and I have tested a patch based on the explanation in the second thread. This has been working flawlessly for approximately a month. I'd really like to see this merged so I don't have to repatch it every time.

@nikwrt
Copy link
Contributor Author

nikwrt commented Jul 29, 2016

Hi, I have been running this now for approximately six months, with no issues whatsoever. Please, consider merging it.

@cmouse
Copy link
Contributor

cmouse commented Jul 30, 2016

I'll try to have a look at this next week. Sorry for the delay.

@cmouse
Copy link
Contributor

cmouse commented Aug 4, 2016

We'll do internal review for this

sirainen
sirainen reviewed Aug 4, 2016
View reviewed changes
src/plugins/fts-squat/fts-backend-squat.c
@@ -137,6 +147,9 @@ fts_backend_squat_set_box(struct squat_fts_backend *backend,
if (backend->full_len != 0)
squat_trie_set_full_len(backend->trie, backend->full_len);
backend->box = box;
ret = squat_trie_open(backend->trie);
if (ret < 0)
return -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing return value for ret >= 0 case.

Copy link
Contributor Author

@nikwrt nikwrt Aug 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended. If ret >= 0 the next line will return what the original code did:

return squat_trie_get_last_uid(backend->trie, last_uid_r);

Previously,fts_backend_squat_set_box could fail and squat_trie_get_last_uid would still be called, originating the error.

Hope this helps...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you are missing return value from the function entirely when ret >= 0, so it'll now more or less randomly return something. I'd guess the above should be: return squat_trie_open(backend->trie);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am puzzled, are we looking at the same code? Please click maybe on "View full changes". If ret >=0, the next line will execute, and that is:

return squat_trie_get_last_uid(backend->trie, last_uid_r);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're looking at fts_backend_squat_get_last_uid(), I'm looking at fts_backend_squat_set_box().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I see it now. You are right. It should return 0 and I overlooked it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, should I fix it in my fork and resubmit it? Just let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that'd be easier for us :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's done now and I see it's been picked up automatically by the pull request. Thanks for pointing it out.

@sirainen
Copy link
Contributor

merged

@sirainen sirainen closed this Aug 16, 2016
DovecotSync pushed a commit that referenced this pull request Jul 9, 2019
@stephanbosch @sirainen
lib-http: guard against hshared use-after-free
6236f36 
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
DovecotSync pushed a commit that referenced this pull request Jul 9, 2019
@stephanbosch
lib-http: guard against hshared use-after-free
4083c73 
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
DovecotSync pushed a commit that referenced this pull request Jul 12, 2019
@stephanbosch
lib-http: guard against hshared use-after-free
7896f07 
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
cgzones added a commit to cgzones/core that referenced this pull request Aug 12, 2024
@cgzones
test-istream-qp-decoder: fix out-of-bounds read
372ba64 
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")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nikwrt @cmouse @sirainen