New CLI arguments and experimental code coverage

[jtpereyda]

Lots of changes bundled together. This was mostly for the experimental code coverage, but I made lots of fixes along the way that are also bundled.

1. New CLI arguments:
   1. --stdout: hide/capture/mirror stdout and stderr
   2. --qemu, --qemu-path: experimental code coverage feedback
   3. --web-port: web GUI port
   4. --restart-interval: restart every n test cases
   5. --target-start-wait: adjustable process startup wait time (default 0). Also removed duplicate wait times sprinkled throughout the code.
2. Experimental code coverage using afl-qemu-trace
   1. Includes some experimental web UI stats
3. TCP connections:
   1. Safer shutdown approach (shutdown, recv, close) -- should improve stability
4. edge.py: Human-readable id instead of integer id.
5. Session:
   1. Add register_post_start_target_callback(): callback after a target is started or restarted.
   2. Code coverage support
   3. stop target even when exceptions are raised
   4. keep web interface open even when exceptions are raised

[jtpereyda]

Edit: Moved to inline comment on tcp_socket_connection.py:119 below.

[jtpereyda]

While working with boofuzz, I realized that the past design choice to return b"" on timeout, combined with the fact that the underlying socket API has recv return 0 to indicated a closed connection, leads to an ambiguity: a return value of b"" can mean a closed connection, or a timeout.

My solution was to throw an exception to indicate the socket has shut down, which is more intuitive to me personally. However, this actually flips the semantics of recv, which throws socket.timeout on timeout and returns b"" on socket shutdown. Perhaps it would be wiser to follow this approach to map better to peoples' existing knowledge.

Either way, we have some choice to make:

1. Keep these changes (throw exception on socket shutdown), which removes the ambiguity but can break existing fuzzers that depend on socket shutdowns yielding an empty bytes array.
2. Flip the approach, that is, allow socket.timeout to be bubbled up as an exception. This removes the ambiguity, but breaks existing fuzzers that depend on on timeouts yielding an empty bytes array.
3. Keep the existing behavior, and add a new way to detect whether the socket was shutdown or whether a timeout happened. This is pretty lame but backwards compatible.

Edit: This change seems to be where the test failures are coming from.

[jtpereyda]

On the other hand, I don't know how many scripts out there would actually hit this compatibility problem.

[jtpereyda]

Perhaps one tool to compare with is pwntools, which also has a recv method for which timeout returns an empty bytes, and a closed connection yields an exception: https://docs.pwntools.com/en/stable/tubes.html#pwnlib.tubes.tube.tube.recv

[SR4ven]

Hmm difficult choice.
I don't like the existing behaviour because we can't differentiate a close from a timeout. So I'd say either stick to the default socket/posix behaviour or raise an exception for everything.

The default posix way would be choice 2 if I understand correctly. Raise socket.timeout and return b"" on close.

However, as we are on application level I could also imagine raising an exception for both cases. Not sure what others think about this but it might be the most user friendly. You simply tell boofuzz to receive after a test case and if anything goes wrong (close/timeout/abort/reset) you get an exception.
On the other hand, I know some of my targets close the connection if the received data was invalid. In that case the target didn't crash but boofuzz would raise an exception. That might be a bit inconvenient.

I also just found the session option check_data_received_each_request. It seems this option is the reason for both close and timeout returning b"". If we switch to exceptions, we'd have to catch them here and depending on the setup maybe re-raise?

Right now I favour choice 2 as anyone with basic knowledge about sockets will understand what's going on. Also it looks like that approach could easily be adapted to the current check_data_received_each_request behaviour.

BTW how can we get EWOULDBLOCK if we use a blocking socket and catch socket.timeout before?

boofuzz/boofuzz/connections/tcp_socket_connection.py

Line 131 in 68ad063

elif e.errno == errno.EWOULDBLOCK: # timeout condition if using SO_RCVTIMEO or SO_SNDTIMEO

And why do we interpret ETIMEDOUT as BoofuzzTargetConnectionReset?

boofuzz/boofuzz/connections/tcp_socket_connection.py

Line 129 in 68ad063

elif (e.errno == errno.ECONNRESET) or (e.errno == errno.ENETRESET) or (e.errno == errno.ETIMEDOUT):

Edit: I wouldn't worry about breaking backwards compatibility. As you already said I doubt that many scripts use the boofuzz socket interface directly and rely on this specific behaviour.

[jtpereyda]

I also just found the session option check_data_received_each_request. It seems this option is the reason for both close and timeout returning b"". If we switch to exceptions, we'd have to catch them here and depending on the setup maybe re-raise?

Yes, that Session option is a layer on top of the socket behavior. The only place in my scripts this would matter is in callbacks, where I sometimes have code set up to receive the next (typically valid) message. Whatever choice we make here, we can modify Session to still act the same way with check_data_received_each_request.

[jtpereyda]

Trying to get all these options clear in my head:

 ----------+--------------+-----------------------+--------------+------------+------------
|          | OS socket    | OS socket nonblocking | boo previous | PR initial | PR propsed |
+----------+--------------+-----------------------+--------------+------------+------------+
| timeout  | wait forever | exception             | return b""   | return b"" | exception  |
| shutdown | return b""   | return b""            | return b""   | exception  | return b"" |
 ----------+--------------+-----------------------+--------------+------------+------------

Part of me wants to yield an exception in both cases. I'm leaning toward matching OS socket behavior, but that behavior is a bit counterintuitive.

[SR4ven]

The table seems to be correct.

Agreed, I feel exactly the same way. I think if you have worked with sockets before, the OS socket behaviour is more intuitive.
It's the other way around if you haven't I guess.

[SR4ven]

Which timeout/shutdown behaviour are we going to use now? The one initially proposed in this PR or the OS like?

[jtpereyda]

@SR4ven The OS-like behavior. Just realized I didn't add the timeout exception though...

[SR4ven]

Alright. We don't need BoofuzzTargetConnectionShutdown anymore do we.

Also, do we need to adapt other connection classes to the new behaviour? UDP maybe?
We could do that in another PR if needed.

[SR4ven]

Nice big change! Looks good so far but I haven't done any testing yet. Do you have an example use case or some documentation on how to use the code coverage feature?

[SR4ven]

Hmm difficult choice.
I don't like the existing behaviour because we can't differentiate a close from a timeout. So I'd say either stick to the default socket/posix behaviour or raise an exception for everything.

The default posix way would be choice 2 if I understand correctly. Raise socket.timeout and return b"" on close.

However, as we are on application level I could also imagine raising an exception for both cases. Not sure what others think about this but it might be the most user friendly. You simply tell boofuzz to receive after a test case and if anything goes wrong (close/timeout/abort/reset) you get an exception.
On the other hand, I know some of my targets close the connection if the received data was invalid. In that case the target didn't crash but boofuzz would raise an exception. That might be a bit inconvenient.

I also just found the session option check_data_received_each_request. It seems this option is the reason for both close and timeout returning b"". If we switch to exceptions, we'd have to catch them here and depending on the setup maybe re-raise?

Right now I favour choice 2 as anyone with basic knowledge about sockets will understand what's going on. Also it looks like that approach could easily be adapted to the current check_data_received_each_request behaviour.

BTW how can we get EWOULDBLOCK if we use a blocking socket and catch socket.timeout before?

boofuzz/boofuzz/connections/tcp_socket_connection.py

Line 131 in 68ad063

elif e.errno == errno.EWOULDBLOCK: # timeout condition if using SO_RCVTIMEO or SO_SNDTIMEO

And why do we interpret ETIMEDOUT as BoofuzzTargetConnectionReset?

boofuzz/boofuzz/connections/tcp_socket_connection.py

Line 129 in 68ad063

elif (e.errno == errno.ECONNRESET) or (e.errno == errno.ENETRESET) or (e.errno == errno.ETIMEDOUT):

Edit: I wouldn't worry about breaking backwards compatibility. As you already said I doubt that many scripts use the boofuzz socket interface directly and rely on this specific behaviour.

[jtpereyda]

Nice big change! Looks good so far but I haven't done any testing yet. Do you have an example use case or some documentation on how to use the code coverage feature?

Here's an example using the CLI:

python /home/jpereyda/code/boofuzz-ftp/ftp.py fuzz --web-port 26000 --target 127.0.0.1:2200 --target-cmd '/home/jpereyda/code/LightFTP/Source/Release/fftp /home/jpereyda/code/LightFTP/Source/Release/fftp.conf' --stdout hide --restart-interval 32000 --target-start-wait 3.5 --qemu ftp --username ubuntu --password ubuntu

This is with the boofuzz-ftp cli-main branch: https://github.com/jtpereyda/boofuzz-ftp/tree/cli-main and target LightFTP. The coverage approach should work on Linux against most binaries, anything that can run in Qemu.

The relevant command line args are the --qemu switch, and the --target-cmd which specifies the target binary.

[jtpereyda]

TODO:

1. Failing Windows tests

Edit: Done

[jtpereyda]

@SR4ven Good to merge this one? The remaining test failures should resolve with your PR.

I gave up on the whole 2.7 compatibility thing. :/

[SR4ven]

So far so good. I left one more question about which socket behaviour we’re going to use.

I didn’t find time to run the example code yet, but I’ll try to do that today. Shouldn’t stop you from merging.

[SR4ven]

There are still some stickler warnings. Some seem to be false positives.