jtpereyda / boofuzz

A fork and successor of the Sulley Fuzzing Framework
GNU General Public License v2.0
2.02k stars 343 forks source link

Block "dep_value" Request is type bytes #661

Closed huntergregal closed 1 year ago

huntergregal commented 1 year ago

Report

Consider this example:

def define_proto(session):
    req = Request("HTTP-Request", children=(
        Block("Request-Line", children=(
            Group(name="Method", values=["GET", "POST"]),
            Delim(name="space-1", default_value=" ", fuzzable=False),
            Block("URI-POST", dep='Method', dep_value='POST', dep_compare='==', children=(
                Group(name="URI-post_", values=["/api/end1", "/api/end2"]),
            )),
            Block("URI-GET", dep='Method', dep_value='GET', dep_compare='==', children=(
                Static(name="URI-get_", default_value="/"),
            )),
            String(name="space-2", default_value=" ", fuzzable=False),
            Group(name="HTTP-Version", values=["HTTP/1.1", "HTTP/1.0"]),
            String(name="req-CRLF", default_value="\r\n", fuzzable=False),
        )),
        Static(name="CRLF-end", default_value="\r\n"),
    ))
    session.connect(req)

The two URI blocks depend on the value of the HTTP Request Method which is a Group with two values of GET or POST.

You would expect the above code to change the requested path depending on if the request is a GET or POST. However this is NOT the case.

After some debugging I found the issue is that at https://github.com/jtpereyda/boofuzz/blob/master/boofuzz/blocks/block.py#L84 dependent_value is actually type(bytes) instead of type(str).

The proper code to make the above example is:

def define_proto(session):
    req = Request("HTTP-Request", children=(
        Block("Request-Line", children=(
            Group(name="Method", values=["GET", "POST"]),
            Delim(name="space-1", default_value=" ", fuzzable=False),
            Block("URI-POST", dep='Method', dep_value=b'POST', dep_compare='==', children=(
                Group(name="URI-post_", values=["/api/end1", "/api/end2"]),
            )),
            Block("URI-GET", dep='Method', dep_value=b'GET', dep_compare='==', children=(
                Static(name="URI-get_", default_value="/"),
            )),
            String(name="space-2", default_value=" ", fuzzable=False),
            Group(name="HTTP-Version", values=["HTTP/1.1", "HTTP/1.0"]),
            String(name="req-CRLF", default_value="\r\n", fuzzable=False),
        )),
        Static(name="CRLF-end", default_value="\r\n"),
    ))
    session.connect(req)

Notice that dep_value has been changed to a bytes type so that the == compare will work as expected.

Based on the fact that the 2 optional values passed to the Method group were strings, not bytes, I consider this a bug or at best - a failure of documentation.

Expected behavior

No response

Actual behavior

No response

Steps to reproduce the problem

from boofuzz import *
def define_proto(session):
    req = Request("HTTP-Request", children=(
        Block("Request-Line", children=(
            Group(name="Method", values=["GET", "POST"]),
            Delim(name="space-1", default_value=" ", fuzzable=False),
            Block("URI-POST", dep='Method', dep_value='POST', dep_compare='==', children=(
                Group(name="URI-post_", values=["/api/end1", "/api/end2"]),
            )),
            Block("URI-GET", dep='Method', dep_value='GET', dep_compare='==', children=(
                Static(name="URI-get_", default_value="/"),
            )),
            String(name="space-2", default_value=" ", fuzzable=False),
            Group(name="HTTP-Version", values=["HTTP/1.1", "HTTP/1.0"]),
            String(name="req-CRLF", default_value="\r\n", fuzzable=False),
        )),
        Static(name="CRLF-end", default_value="\r\n"),
    ))
    session.connect(req)

def main():
    session = Session(
        target=Target(
            connection=TCPSocketConnection("127.0.0.1", 50000)
        ),
    )

    define_proto(session=session)

    session.fuzz()

main()
jtpereyda commented 1 year ago

@huntergregal Thank you for the bug report! Seems like a doc fix is the quickest update. An added step could be a type check in the Block constructor.

I'm working from memory, but I think every block/primitive renders to bytes, so it really does make sense for dep_value to be of type bytes as well.