Group primitive: Value of default_value cannot be used in fuzzing

[lukem1]

Report

See comment below, which better identifies the issue.

When using the Group() primitive the value of default_value is removed from the values list:

boofuzz/boofuzz/primitives/group.py

Lines 36 to 37 in dc9fc92

	if default_value in values:
	values.remove(default_value)

This prevents the value from being used in fuzzing even if the user explicitly includes the value in the values list as seen in the below example:

Group(name="Method", default_value="GET", values=["GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE"])

Further, if the default_value parameter is unset it will automatically be set to values[0], then the value will be removed from the values list. This behavior is not evident from the Group() docstring and leads to unexpected fuzzing behavior.

For instance, the primary protocol definition example in the BooFuzz documentation is impacted by this unexpected behavior:

boofuzz/examples/http_simple.py

Lines 22 to 38 in dc9fc92

	req = Request("HTTP-Request", children=(
	Block("Request-Line", children=(
	Group(name="Method", values=["GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE"]),
	Delim(name="space-1", default_value=" "),
	String(name="URI", default_value="/index.html"),
	Delim(name="space-2", default_value=" "),
	String(name="HTTP-Version", default_value="HTTP/1.1"),
	Static(name="CRLF", default_value="\r\n"),
	)),
	Block("Host-Line", children=(
	String(name="Host-Key", default_value="Host:"),
	Delim(name="space", default_value=" "),
	String(name="Host-Value", default_value="example.com"),
	Static(name="CRLF", default_value="\r\n"),
	)),
	Static(name="CRLF", default_value="\r\n"),
	))

When using the above example the HTTP GET method will never be used during fuzzing.

Expected behavior

No response

Actual behavior

No response

Steps to reproduce the problem

Using the provided BooFuzz script:

1. Start a webserver to test against

python3 -m http.server

2. Execute the provided BooFuzz script, ensuring that the TCP connection uses the appropriate port

python3 boofuzz_http.py

3. Note that a BooFuzz testing step is created for the POST method, but not the GET method. This is the case both when the default_value is set to GET and when it is not set at all.

[2023-12-29 12:14:50,188]     Info: Web interface can be found at http://localhost:26000
[2023-12-29 12:14:50,189] Test Case: 1: HTTP-Request:[HTTP-Request.Request-Line.Method:0]
[2023-12-29 12:14:50,189]     Info: Type: Group
[2023-12-29 12:14:50,189]     Info: Opening target connection (0.0.0.0:8000)...
[2023-12-29 12:14:50,189]     Info: Connection opened.
[2023-12-29 12:14:50,189]   Test Step: Monitor CallbackMonitor#140115225371664[pre=[],post=[],restart=[],post_start_target=[]].pre_send()
[2023-12-29 12:14:50,189]   Test Step: Fuzzing Node 'HTTP-Request'
[2023-12-29 12:14:50,189]     Info: Sending 48 bytes...
[2023-12-29 12:14:50,189]     Transmitted 48 bytes: 50 4f 53 54 20 2f 69 6e 64 65 78 2e 68 74 6d 6c 20 48 54 54 50 2f 31 2e 31 0d 0a 48 6f 73 74 3a 20 65 78 61 6d 70 6c 65 2e 63 6f 6d 0d 0a 0d 0a b'POST /index.html HTTP/1.1\r\nHost: example.com\r\n\r\n'
[2023-12-29 12:14:50,189]   Test Step: Contact target monitors
[2023-12-29 12:14:50,189]   Test Step: Cleaning up connections from callbacks
[2023-12-29 12:14:50,189]       Check OK: No crash detected.
[2023-12-29 12:14:50,189]     Info: Closing target connection...
[2023-12-29 12:14:50,189]     Info: Connection closed.

Fuzzing session completed. Keeping webinterface up on localhost:26000 
Press ENTER to close webinterface

boofuzz script

from boofuzz import *

session = Session(
    target=Target(
        connection=TCPSocketConnection("0.0.0.0", 8000)
    )
)

req = Request("HTTP-Request", children=(
        Block("Request-Line", children=(
            Group(name="Method", default_value="GET", values=["GET", "POST"]),
            Delim(name="space-1", default_value=" ", fuzzable=False),
            String(name="URI", default_value="/index.html", fuzzable=False),
            Delim(name="space-2", default_value=" ", fuzzable=False),
            String(name="HTTP-Version", default_value="HTTP/1.1", fuzzable=False),
            Static(name="CRLF", default_value="\r\n"),
        )),
        Block("Host-Line", children=(
            String(name="Host-Key", default_value="Host:", fuzzable=False),
            Delim(name="space", default_value=" ", fuzzable=False),
            String(name="Host-Value", default_value="example.com", fuzzable=False),
            Static(name="CRLF", default_value="\r\n"),
        )),
        Static(name="CRLF", default_value="\r\n"),
    ))

session.connect(req)
session.fuzz()

boofuzz version

0.4.2

Python version

3.10

Platform

Linux

Anything else?

No response

[lukem1]

After investigating this a bit more I see that the default value is used if more than one node is being fuzzed, but that no test case is produced for the "default case". For example:

testRequestOne = Request("test-request",
    children=(
        Block("test-block",
              children=(
                Group("AB", values=["a", "b"], fuzzable=True),
                Group("XY", values=["x", "y"], fuzzable=True)
            )
        )
    )
)

Will produce the following test cases: bx, by, ay, by

Note that the ax case is missing. This is a much smaller issue than I initially thought, but I still feel that the default case should be included in testing.

The duplicates are also unexpected as that will scale poorly. I see that has been discussed ie in #622