Skip to content
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

Undefine-shift in sldns_str2wire_hip_buf #610

Closed
JiangHeng12138 opened this issue Jan 24, 2022 · 1 comment
Closed

Undefine-shift in sldns_str2wire_hip_buf #610

JiangHeng12138 opened this issue Jan 24, 2022 · 1 comment

Comments

@JiangHeng12138
Copy link

version: unbound-1.11.0

sldns/str2wire.c:2080:10: runtime error: left shift of negative value -524206
    #0 0x5d039c in sldns_str2wire_hip_buf /src/unbound/sldns/str2wire.c
    #1 0x4b2400 in LLVMFuzzerTestOneInput /src/unbound/fuzz_3.c:58:17
    #2 0x443833 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
    #3 0x42efa2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:6
    #4 0x434c46 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:9
    #5 0x45e152 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #6 0x7f4a60869b26 in __libc_start_main (/lib64/libc.so.6+0x25b26)
    #7 0x40ae59 in _start (/root/oss-fuzz/build/out/unbound/fuzz_3_fuzzer+0x40ae59)
@wcawijngaards
Copy link
Member

Took some effort to figure this out, but it seems that a right shift is not using unsigned, but a signed type. By casting to unsigned before that shift, the negative value should go away. The fix has that. It also masks the added value, which is similar to issue #611, so that it cannot integer overflow during the addition of the calculation.

This error could possibly have resulted in a wrong error code or wrong position of the error. The fix should fix it, but I have no reproducer case, the commit applies to the 1.11.0 codebase, or you could use the current repo, if you want to try that.

wcawijngaards added a commit that referenced this issue Jan 25, 2022
- Fix #610: Undefine-shift in sldns_str2wire_hip_buf.
jedisct1 added a commit to jedisct1/unbound that referenced this issue Jan 25, 2022
* nlnet/master:
  Changelog note for NLnetLabs#612: - Merge PR NLnetLabs#612: TCP race condition.
  - Fix NLnetLabs#588: Unbound 1.13.2 crashes due to p->pc is NULL in   serviced_udp_callback.
  - Better bookkeeping when reclaiming the TCP buffer.
  - Mark waiting_tcp and serviced_query as being in the   cb_and_decommission stage to signal later code about their state;   prevents premature item deletion.
  Changelog note for NLnetLabs#610 - Fix NLnetLabs#610: Undefine-shift in sldns_str2wire_hip_buf.
  - Fix NLnetLabs#610: Undefine-shift in sldns_str2wire_hip_buf.
  - Add serviced_query timer to send upstream queries outside of the mesh   flow to prevent race conditions.
  - For dnstap, do not wakeupnow right there. Instead zero the timer to   force the wakeup callback asap.
  - For NLnetLabs#602: Allow the module-config "subnetcache validator cachedb   iterator".
  - Add rpz: for-downstream: yesno option, where the RPZ zone is   authoritatively answered for, so the RPZ zone contents can be   checked with DNS queries directed at the RPZ zone.
  Changelog note for NLnetLabs#605: - Merge PR NLnetLabs#605: Fix EDNS to upstream where the same option could be   attached more than once.
  - Make sure callback changes for EDNS are not lost.
  - Fix EDNS to upstream where the same option could be attached more than   once. - Add a region to serviced_query for allocations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants