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

Integer Overflow in sldns_str2period function #637

Closed
kobrineli opened this issue Feb 28, 2022 · 3 comments
Closed

Integer Overflow in sldns_str2period function #637

kobrineli opened this issue Feb 28, 2022 · 3 comments

Comments

@kobrineli
Copy link

I've got an input to reach unsigned integer overflow error in sldns/parseutil.c:272 in sldns_str2period function, that leads to incorrect conversation from string to uint32_t. Also, other arithmetic in this function can lead to unsigned integer overflow. I have not found any checks to prevent this error, is that alright?

Steps to reproduce the behavior:

  1. Build docker container for unbound from https://github.com/ispras/oss-sydr-fuzz/tree/master/projects/unbound
  2. Run docker container and execute /unbound_fuzzers/fuzz_3_fuzzer overflow_input.txt with this input:
    overflow_input.txt
  3. You will see something like this:
    sldns/parseutil.c:272:7: runtime error: unsigned integer overflow: 440000633 * 10 cannot be represented in type 'unsigned int'
  • Unbound version: 7749d98
  • OS: ubuntu 20.04
@wcawijngaards
Copy link
Member

What the function does today does not seem a problem, it parses and returns something like the lower 32bits of the end result. It does not check for the number itself, if that exceeds max 32bit value. That is a lack of error reporting.

@kobrineli
Copy link
Author

Ok. I don't know all the details of code, won't this wraparound be critical for some time measurements and other results, is that okay that error might happen? By the way, sldns_str2period function was called sldns_str2wire_period_buf function.

@wcawijngaards
Copy link
Member

The commit fixes it by returning an error when there is an integer overflow in the function. The parse can then stop and print an error, to the user.

jedisct1 added a commit to jedisct1/unbound that referenced this issue Mar 14, 2022
* nlnet/master: (27 commits)
  Changelog note for NLnetLabs#644, move commands together for library binary.
  Make `install-lib` make target install the pkg-config file.
  - Fix configure for python to use sysutils, because distutils is   deprecated. It uses sysutils when available, distutils otherwise.
  - Fix for NLnetLabs#637: fix integer overflow checks in sldns_str2period.
  - Fix NLnetLabs#637: Integer Overflow in sldns_str2period function.
  - Fix compile warnings for printf ll format on mingw compile.
  - Various fixes for NLnetLabs#632: variable initialisation, convert the qinfo   to str once, accept trailing dot in the local-zone ipset option.
  Changelog entry for NLnetLabs#632 - Merge PR NLnetLabs#632 from scottrw93: Match cnames in ipset.
  - Added tests for ipset.
  - Fix pythonmod for change in iter_dp_is_useless function prototype.
  - Fix for edns client subnet option add fix in removal code, from review.
  - Fix edns client subnet to add the option based on the option list,   so that it is not state dependent, after the state fix of NLnetLabs#605 for   double EDNS options.
  Changelog entry for NLnetLabs#623: - Merge NLnetLabs#623 from rex4539: Fix typos.
  - Fix NLnetLabs#630: Unify the RPZ log messages.
  - Fix for NLnetLabs#633: updated fix with new text.
  - Fix NLnetLabs#633: Document unix domain socket support for unbound-control.
  - Fix check interface existence for support detection in remote lookup.
  - update Makefile dependencies.
  - Fix to detect that no IPv6 support means that IPv6 addresses are   useless for delegation point lookups.
  Match cnames in ipset
  ...
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