-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Protodetect enip dns 4388 v7 #6185
Conversation
Strict length for register sessions NOP command must have options=0
Checks opcode is valid Checks additional_rr do not exceed message length Better logic for incomplete cases
Checks credentials flavor is known
27e2809
to
f355941
Compare
if header.additional_rr as usize | ||
+ header.answer_rr as usize | ||
+ header.authority_rr as usize | ||
+ header.questions as usize | ||
> rlen | ||
{ | ||
//not enough data for additional records | ||
return (false, false, false); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As these are counts not sizes, its not accurate, but also won't false positive. Should the size of the header be calculated in as well? If sizeof(header) + addtional_rr +answer_rr > rlen... Might be a bit more accurate. I think we could also assume a minumum of 2 bytes per expected record being the CLASS is a u16.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, these are counts, not sizes.
And I do not count the header size.
But as each of these has at least one byte, we can do this test to exclude some cases
So, should I include the header size in this sum ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. Just to confirm, rlen is the size of the record. In TCP this would be the first 2 bytes, in UDP its the payload size.
So a test to make sure its greater than or equal to:
sizeof(header) + (additional_rr * 2) + (answer_rr * 2) + (authority_rr * 2) + (questions * 2)
should be safe and accurate, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. Just to confirm, rlen is the size of the record. In TCP this would be the first 2 bytes, in UDP its the payload size.
Exactly, you can check the callers do it right
So a test to make sure its greater than or equal to:
sizeof(header) + (additional_rr * 2) + (answer_rr * 2) + (authority_rr * 2) + (questions * 2)
should be safe and accurate, right?
Ok, will do this
Replaced by #6195 |
Previously, the problem was that nested headers/boundaries were not used to compute the hash Solution is to move up the call to the hash computation from ProcessMimeBody to its caller ProcessMimeEntity, and add a set of conditions to ensure that we are not in the principal headers. Ticket: OISF#6185
Previously, the problem was that nested headers/boundaries were not used to compute the hash Solution is to move up the call to the hash computation from ProcessMimeBody to its caller ProcessMimeEntity, and add a set of conditions to ensure that we are not in the principal headers. Ticket: OISF#6185
Previously, the problem was that nested headers/boundaries were not used to compute the hash Solution is to move up the call to the hash computation from ProcessMimeBody to its caller ProcessMimeEntity, and add a set of conditions to ensure that we are not in the principal headers. Ticket: OISF#6185
Previously, the problem was that nested headers/boundaries were not used to compute the hash Solution is to move up the call to the hash computation from ProcessMimeBody to its caller ProcessMimeEntity, and add a set of conditions to ensure that we are not in the principal headers. Ticket: OISF#6185
Previously, the problem was that nested headers/boundaries were not used to compute the hash Solution is to move up the call to the hash computation from ProcessMimeBody to its caller ProcessMimeEntity, and add a set of conditions to ensure that we are not in the principal headers. Ticket: OISF#6185
Previously, the problem was that nested headers/boundaries were not used to compute the hash Solution is to move up the call to the hash computation from ProcessMimeBody to its caller ProcessMimeEntity, and add a set of conditions to ensure that we are not in the principal headers. Ticket: OISF#6185
Previously, the problem was that nested headers/boundaries were not used to compute the hash Solution is to move up the call to the hash computation from ProcessMimeBody to its caller ProcessMimeEntity, and add a set of conditions to ensure that we are not in the principal headers. Ticket: OISF#6185 (cherry picked from commit a3168fd)
Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/4388
Describe changes:
so as to avoid protocol detection confusion with TCP splitting
Modifies #6182 with
LIST_INTERFACES
(found by fuzzing the fixing branch)