Bug 1301 - stack-buffer-overflow on address found with address sanitizer
stack-buffer-overflow on address found with address sanitizer
Status: RESOLVED FIXED
Product: unbound
Classification: Unclassified
Component: server
1.6.2
x86_64 Linux
: P5 major
Assigned To: unbound team
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-16 04:02 CEST by Erik
Modified: 2017-06-16 09:32 CEST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Erik 2017-06-16 04:02:15 CEST
Building and testing on Debian testing x86_64 with AddressSanitizer:
```
CFLAGS="-fsanitize=address -g" CXXFLAGS=$CFLAGS ./configure && make all check
````
finds the following stack-buffer-overflow
```
./unittest
Start of unbound 1.6.4 unit test.
test authzone functions
=================================================================
==16191==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffee8d4bfa0 at pc 0x55a32a6cdd99 bp 0x7ffee8d4be80 sp 0x7ffee8d4be78
READ of size 4 at 0x7ffee8d4bfa0 thread T0
    #0 0x55a32a6cdd98 in hashlittle util/storage/lookup3.c:375
    #1 0x55a32a7605e9 in rrset_key_hash util/data/packed_rrset.c:170
    #2 0x55a32a665392 in auth_packed_rrset_copy_region services/authzone.c:139
    #3 0x55a32a665d1c in msg_add_rrset_an.lto_priv.93 services/authzone.c:192
    #4 0x55a32a64ca8a in az_generate_positive_answer services/authzone.c:1989
    #5 0x55a32a64e560 in az_generate_answer_with_node services/authzone.c:2243
    #6 0x55a32a64ee7d in auth_zone_generate_answer services/authzone.c:2338
    #7 0x55a32a64f1aa in auth_zones_lookup services/authzone.c:2366
    #8 0x55a32a78cef1 in q_ans_query testcode/unitauth.c:752
    #9 0x55a32a78d8f9 in check_az_q_ans testcode/unitauth.c:807
    #10 0x55a32a78daed in check_queries testcode/unitauth.c:829
    #11 0x55a32a78dc20 in authzone_query_test testcode/unitauth.c:847
    #12 0x55a32a78dc48 in authzone_test testcode/unitauth.c:857
    #13 0x55a32a7aecaf in main testcode/unitmain.c:872
    #14 0x7fc7b2c6e2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
    #15 0x55a32a5f96e9 in _start (/home/erikd/Git/unbound/unittest+0x526e9)

Address 0x7ffee8d4bfa0 is located in stack of thread T0 at offset 32 in frame
    #0 0x55a32a7604b0 in rrset_key_hash util/data/packed_rrset.c:163
```
Comment 1 Erik 2017-06-16 04:06:28 CEST
Forgot to mention that was building from the Git repository https://github.com/jedisct1/unbound which seems to be an up-to-date mirror of your SVN repo.
Comment 2 Wouter Wijngaards 2017-06-16 08:44:16 CEST
Hi Erik,

Yes the hashlittle reads beyond bounds (read up on that in lookup3.c around line 375).  It does so on purpose for speed reasons; but won't actually use the bad data or cause a segmentation error.

There is an #ifdef there to make purifiers happy, if you want that.

So it seems to be false positive.  Thank you for the report!

Best regards, Wouter
Comment 3 Erik 2017-06-16 09:07:09 CEST
I'm ok with this specific issue found using AddressSantizier being a WONTFIX, but i also think that if you are not using AddressSanitizer as part of your normal development process, you will end up with bugs.

The fact that you suggest defining VALGRIND to disable this warning shows that you currently don't use ASAN, which in my experience finds far more bugs than Valgrind and has a smaller impact on performance.

For instance if I compile with:

    CFLAGS="-fsanitize=address -g -DVALGRIND" CXXFLAGS=$CFLAGS ./configure

I now get ASAN reporting numerous memory leaks in `unittest`:
```
=================================================================
==18423==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0x7f18bf794ed0 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1ed0)
    #1 0x562bca538b40 in views_create services/view.c:59
    #2 0x562bca5e0004 in respip_view_conf_actions_test testcode/unitmain.c:683
    #3 0x562bca5e18e8 in respip_test testcode/unitmain.c:824
    #4 0x562bca5e19ea in main testcode/unitmain.c:875
    #5 0x7f18bea382b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0x7f18bf794ed0 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1ed0)
    #1 0x562bca538b40 in views_create services/view.c:59
    #2 0x562bca5e141d in respip_view_conf_data_test testcode/unitmain.c:801
    #3 0x562bca5e18de in respip_test testcode/unitmain.c:822
    #4 0x562bca5e19ea in main testcode/unitmain.c:875
    #5 0x7f18bea382b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x7f18bf794ed0 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1ed0)
    #1 0x562bca5e123a in respip_view_conf_data_test testcode/unitmain.c:795
    #2 0x562bca5e18de in respip_test testcode/unitmain.c:822
    #3 0x562bca5e19ea in main testcode/unitmain.c:875
    #4 0x7f18bea382b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

```
Those are almost certainly leaks from the test program rather than the actual unbound library, but their existence makes it harder to validate unbound using tools like the sanitizers because the un-important/irrelevant warnings here make testing with ASAN (and tools like fuzzers) difficult.
Comment 4 Wouter Wijngaards 2017-06-16 09:32:37 CEST
Hi Erik,

Thanks for the tests, compiled with asan, and fixed a couple memory leaks in the new response-based-IP address code and edns-subnet.

Best regards, Wouter