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

Fix potential overflow bug while parsing port in function cfg_mark_ports #1062

Merged
merged 1 commit into from
May 7, 2024

Conversation

xiaoxiaoafeifei
Copy link
Contributor

@xiaoxiaoafeifei xiaoxiaoafeifei commented May 7, 2024

Fix potential overflow bugs caused by the following situations: use strtol function instead of atoi function

  1. The string to be parsed may contain non-numeric characters.
  2. The port parsed from the string may be negative.

Copy link
Member

@wcawijngaards wcawijngaards left a comment

Choose a reason for hiding this comment

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

Nice to check for more error conditions. I cannot spot where it would have had negative numbers from the string.

@wcawijngaards wcawijngaards merged commit 49569b8 into NLnetLabs:master May 7, 2024
1 check passed
wcawijngaards added a commit that referenced this pull request May 7, 2024
  and redundant check for array size.
And changelog note for merge of #1062.
@wcawijngaards
Copy link
Member

Thank you for the contribution! The fixup commit moves a declaration before statements to avoid a warning, then avoids printing a null value on error, just in case, and adds a redundant check on array size, also just for extra certainty. The pull request should catch unknown characters in that string and also range errors and that is nice to have.

jedisct1 added a commit to jedisct1/unbound that referenced this pull request May 7, 2024
* nlnet/master: (45 commits)
  - Fix for NLnetLabs#1062: declaration before statement, avoid print of null,   and redundant check for array size. And changelog note for merge of NLnetLabs#1062.
  Fix potential overflow bug while parsing port in function cfg_mark_ports
  - Set version number to 1.20.0 for release.
  - Fix for the DNSBomb vulnerability CVE-2024-33655. Thanks to Xiang Li   from the Network and Information Security Lab of Tsinghua University   for reporting it.
  - Fix doxygen comment for errinf_to_str_bogus.
  - Cleanup unnecessary strdup calls for EDE strings.
  - Man page entry for unbound-checkconf -q.
  - Fix NLnetLabs#876: [FR] can unbound-checkconf be silenced when configuration   is valid?
  - Add unit tests for cachedb and subnet cache expired data.
  - Fix cachedb with serve-expired-client-timeout disabled. The edns   subnet module deletes global cache and cachedb cache when it   stores a result, and serve-expired is enabled, so that the global   reply, that is older than the ecs reply, does not return after   the ecs reply expires.
  - Fix doc unit test for out of directory build.
  - Fix to disable fragmentation on systems with IP_DONTFRAG,   with a nonzero value for the socket option argument.
  Changelog note for NLnetLabs#1041 and NLnetLabs#1038. - Merge NLnetLabs#1041: Stub and Forward unshare. This has one structure   for them and fixes NLnetLabs#1038: fatal error: Could not initialize   thread / error: reading root hints.
  Update locking management for iter_fwd and iter_hints methods. (NLnetLabs#1054)
  - Fix configure flto check error, by finding grep for it.
  - Fix ci workflow for macos for moved install locations.
  - Merge NLnetLabs#1053: Remove child delegations from cache when grandchild   delegations are returned from parent.
  - When a granchild delegation is returned, remove any cached child delegations   up to parent to not cause delegation invalidation because of an   expired child delegation that would never be updated. Most likely to   happen without qname-minimisation. Reported by Roland van Rijswijk-Deij.
  - Fix edns subnet to sort rrset references when storing messages   in the cache. This fixes a race condition in the rrset locks.
  - Add checklock feature verbose_locking to trace locks and unlocks.
  ...
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

Successfully merging this pull request may close these issues.

None yet

2 participants