boofuzz icon indicating copy to clipboard operation
boofuzz copied to clipboard
verified publisher icon jtpereyda

reconnect to the target when socket is broken with reuse_target_connection set

Open cq674350529 opened this issue 6 years ago • 2 comments

The option reuse_target_connection makes it possible to reuse the same target connection, which is useful in some cases. https://github.com/jtpereyda/boofuzz/blob/bad385117c4176c4dde770dab7cacccaed1c5b02/boofuzz/sessions.py#L311-L312

When enabling option reuse_target_connection in a fuzz test, it seems it doesn't work properly. If the socket is broken, the current implementation just ignores it and sends data anyway.

An example of log is as follows.

[2019-04-02 17:15:15,115] Test Case: 44: login1.no-name.44
[2019-04-02 17:15:15,115]     Info: Type: String. Default value: '00-0e-c6-dc-07-bf'. Case 44 of 4282 overall.
[2019-04-02 17:15:15,115]   Test Step: Fuzzing Node 'login1'
[2019-04-02 17:15:15,116]     Info: Sending 968 bytes...
[2019-04-02 17:15:15,116]     Info: Target connection reset.
[2019-04-02 17:15:15,116]     Info: Receiving...
[2019-04-02 17:15:15,116]     Received: 48 54 54 50 2f 31 2e 31 20 32 30 30 20 4f 4b 0d 0a 43 6f 6e 74 65 6e 74 2d 54 79 70 65 3a 20 74 65 78 74 2f 68 74 6d 6c 3b 20 63 68 61 72 73 65 74 3d 75 74 66 2d 38 0d 0a 54 72 61 6e 73 66 65 72 2d 45 6e 63 6f 64 69 6e 67 3a 20 63 68 75 6e 6b 65 64 0d 0a 43 61 63 68 65 2d 43 6f 6e 74 72 6f 6c 3a 20 6e 6f 2d 63 61 63 68 65 0d 0a 50 72 61 67 6d 61 3a 20 6e 6f 2d 63 61 63 68 65 0d 0a 43 6f 6e 6e 65 63 74 69 6f 6e 3a 20 43 6c 6f 73 65 0d 0a 44 61 74 65 3a 20 54 75 65 2c 20 30 32 20 41 70 72 20 32 30 31 39 20 30 39 3a 31 35 3a 31 33 20 47 4d 54 0d 0a 58 2d 46 72 61 6d 65 2d 4f 70 74 69 6f 6e 73 3a 20 53 41 4d 45 4f 52 49 47 49 4e 0d 0a 53 74 72 69 63 74 2d 54 72 61 6e 73 70 6f 72 74 2d 53 65 63 75 72 69 74 79 3a 20 6d 61 78 2d 61 67 65 3d 33 31 35 33 36 30 30 30 3b 20 69 6e 63 6c 75 64 65 53 75 62 44 6f 6d 61 69 6e 73 0d 0a 58 2d 41 67 67 72 65 67 61 74 65 2d 41 75 74 68 3a 20 31 0d 0a 0d 0a 'HTTP/1.1 200 OK\r\nContent-Type: text/html; charset=utf-8\r\nTransfer-Encoding: chunked\r\nCache-Control: no-cache\r\nPragma: no-cache\r\nConnection: Close\r\nDate: Tue, 02 Apr 2019 09:15:13 GMT\r\nX-Frame-Options: SAMEORIGIN\r\nStrict-Transport-Security: max-age=31536000; includeSubDomains\r\nX-Aggregate-Auth: 1\r\n\r\n'
[2019-04-02 17:15:15,116] Test Case: 45: login1.no-name.45
[2019-04-02 17:15:15,117]     Info: Type: String. Default value: '00-0e-c6-dc-07-bf'. Case 45 of 4282 overall.
[2019-04-02 17:15:15,117]   Test Step: Fuzzing Node 'login1'
[2019-04-02 17:15:15,117]     Info: Sending 956 bytes...
[2019-04-02 17:15:15,117]     Info: Target connection reset.
[2019-04-02 17:15:15,117]     Info: Receiving...
[2019-04-02 17:15:15,117]     Received: 0d 0a 30 0d 0a 0d 0a '\r\n0\r\n\r\n'
[2019-04-02 17:15:15,117] Test Case: 46: login1.no-name.46
[2019-04-02 17:15:15,117]     Info: Type: String. Default value: '00-0e-c6-dc-07-bf'. Case 46 of 4282 overall.
[2019-04-02 17:15:15,117]   Test Step: Fuzzing Node 'login1'
[2019-04-02 17:15:15,118]     Info: Sending 939 bytes...
[2019-04-02 17:15:15,118]     Info: Target connection reset.
[2019-04-02 17:15:15,118]     Info: Receiving...
[2019-04-02 17:15:15,118]     Received: 
[2019-04-02 17:15:15,127] Test Case: 47: login1.no-name.47
[2019-04-02 17:15:15,127]     Info: Type: String. Default value: '00-0e-c6-dc-07-bf'. Case 47 of 4282 overall.
[2019-04-02 17:15:15,127]   Test Step: Fuzzing Node 'login1'
[2019-04-02 17:15:15,128]     Info: Sending 946 bytes...
[2019-04-02 17:15:15,128]     Info: Target connection reset.
[2019-04-02 17:15:15,128]     Info: Receiving...
[2019-04-02 17:15:15,128]     Received: 
[2019-04-02 17:15:15,141] Test Case: 48: login1.no-name.48
[2019-04-02 17:15:15,142]     Info: Type: String. Default value: '00-0e-c6-dc-07-bf'. Case 48 of 4282 overall.
[2019-04-02 17:15:15,142]   Test Step: Fuzzing Node 'login1'
[2019-04-02 17:15:15,142]     Info: Sending 993 bytes...
[2019-04-02 17:15:15,143]     Info: Target connection reset.
[2019-04-02 17:15:15,143]     Info: Receiving...
[2019-04-02 17:15:15,144]     Received:

https://github.com/jtpereyda/boofuzz/blob/bad385117c4176c4dde770dab7cacccaed1c5b02/boofuzz/sessions.py#L1094-L1108

The exception will be caught at line 1098.

It's better to recover the socket when the socket is broken.

cq674350529 avatar Apr 03 '19 13:04 cq674350529

@cq674350529 my apologies again for the delay.

Some thoughts (point 3 is probably what you want):

  1. I can't remember the details of when _ignore_connection_issues_when_sending_fuzz_data was first created... but it should probably kill the test case instead of proceeding. This might get messy in the code, which is probably due for a refactor soon.
  2. Furthermore, if a connection dies on your first send in a test case, what we probably really want to do is try reconnecting for a while (and maybe fail if it takes too long). This might obviate point 1, not sure. Note: Some commercial product(s) have a similar behavior to what boofuzz is doing now, which can result in millions of test cases simply skipped. Your example looks like this is happening. Not cool.
  3. As for the actual issue you're focusing on, the immediate fix might be in https://github.com/jtpereyda/boofuzz/blob/bad385117c4176c4dde770dab7cacccaed1c5b02/boofuzz/sessions.py#L1399-1400 If it's possible to check whether a socket is open, then instead of just skipping the open when self._reuse_target_connection, boofuzz could check the socket and re-open if needed.

Regarding point 3: Some initial research shows this isn't super easy with sockets. For the interested: https://stackoverflow.com/a/15175067/461834

A workaround could be to set a flag in the Socket class when an error is detected, then it can be checked at line 1399 mentioned above.... or maybe the flag should be set in the Session class. I don't know, but my intuition says it's closer to the Socket class.

I guess the flag could be called "is_open" which is set when open() is called and cleared when an error occurs or close() is called. There may be some gotchas, but in theory it shouldn't be too bad. "is_open" would be meaningless for UDP though, in which case we would NOT want to "reconnect". This is probably fine if the call to open() does nothing for UDP. To be more precise the flag could be called "connection_failure_detected" or somesuch.

One last note: As the StackOverflow post describes, there is some subtlety since a socket can be half closed. Probably it's fine to just have one error condition for now (sure would be easier anyway :P).

jtpereyda avatar Apr 12 '19 03:04 jtpereyda

@jtpereyda Thanks for your advice. Yes, point 3 is exactly what I want. I'll try and see if I can solve it in my case, maybe add a flag as you advise.

cq674350529 avatar Apr 12 '19 07:04 cq674350529