fix: prevent panic in getHostByName on DNS lookup failure#475
Open
Yanhu007 wants to merge 1 commit intoMasterminds:masterfrom
Open
fix: prevent panic in getHostByName on DNS lookup failure#475Yanhu007 wants to merge 1 commit intoMasterminds:masterfrom
Yanhu007 wants to merge 1 commit intoMasterminds:masterfrom
Conversation
getHostByName discards the error from net.LookupHost and unconditionally indexes into the result slice. When DNS resolution fails, the slice is nil and rand.Intn(0) panics, crashing the entire program. Return the input hostname unchanged when lookup fails or returns no addresses, instead of panicking. Partially addresses Masterminds#473 (Finding 3: CWE-476)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partially addresses #473 (Finding 3: CWE-476)
Problem
getHostByNamediscards the error fromnet.LookupHostand unconditionally indexes into the result slice:When DNS resolution fails (e.g. NXDOMAIN, network error),
addrsis nil, andrand.Intn(0)panics with an unrecoverable crash. The TODO comment acknowledges this was supposed to be fixed.Fix
Check for errors and empty results. Return the input hostname unchanged when lookup fails, instead of panicking:
Testing
All tests pass (one pre-existing timezone-dependent failure in
TestHtmlDateis unrelated).