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

Fuzz rampage! #237

Merged
merged 8 commits into from
Aug 5, 2015
Merged

Fuzz rampage! #237

merged 8 commits into from
Aug 5, 2015

Conversation

FiloSottile
Copy link
Contributor

Multiple patches for crashing bugs found with go-fuzz. Rewrote some sections entirely. Got rid of the rdlen value while parsing.

When a pointer points to a empty name, the "return '.'" special case used to
kick in which is not pointer-aware so it would reset the parsing offset to
the pointer target

This was independently found and fixed in c13d4ee, I'm submitting this patch
anyway as it seems a bit more robust and DRY [citation needed].
Added some more symmetric sanity checks when packing and unpacking, and
simplified the logic a bit, which should still remain mostly unchanged and
incomplete.  A complete implementation of the draft would require context
about whether it's a request or reply and possibly from the request
corresponding to the reply (!£$!@$!) so screw it.
off1 = off
}
return ".", off1, nil
}
Copy link
Owner

Choose a reason for hiding this comment

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

did you rebase on the latest master, or is this piece of code wrong as well? See #234

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is a rebase artifact. The commit message acknowledges that this was independently fixed. Instead of reasoning about the new code I applied the patch I had tested (and rebased well). I'm lazy.

@miekg
Copy link
Owner

miekg commented Aug 5, 2015

I'm in good company (cough bind9 cough). Merging. Will double check other stuff like #234 later.

miekg added a commit that referenced this pull request Aug 5, 2015
@miekg miekg merged commit 8fdd5c3 into miekg:master Aug 5, 2015
@FiloSottile FiloSottile deleted the fuzz-rampage branch August 5, 2015 11:16
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.

2 participants