nul-terminate cookie read from control connection in iperf_accept

[dxbjavid]

• Version of iperf3 (or development branch, such as master or
  3.1-STABLE) to which this pull request applies: master

• Issues fixed (if any): none

• Brief description of code changes (suitable for use as a commit message):

on the server, iperf_accept() reads exactly COOKIE_SIZE bytes from the client's control connection into test->cookie (a char[COOKIE_SIZE]) but never forces a terminating NUL, whereas a well behaved client only sends 36 characters plus a trailing NUL. A peer that sends 37 non-NUL bytes leaves the buffer unterminated, and it is then handled as a C string in iperf_on_connect() (copied into the JSON cookie field and printed via report_cookie), so strlen reads past the array into the following struct members. This terminates the cookie right after it is read so the later string handling stays in bounds.

[bmah888]

Thanks for the bug report and PR! We'll take a look.

[dxbjavid]

Closing this pull request due to inactivity

[davidBar-On]

@dxbjavid I suggest that you will reopen the PR as the issue you found is still an issue.

It may also help if you will changes the fix approach so the effective cookie size will remain 37 bytes. This is by adding a "guarding" null byte after the cookie buffer:

1. Remove your current change and rebase to the latest iperf3 master.
2. In iperf.h change char cookie[COOKIE_SIZE]; to char cookie[COOKIE_SIZE + 1];.
3. Change COOKIE_SIZE to COOKIE_SIZE + 1 in memset(testp->cookie, 0, COOKIE_SIZE); in both iperf_defaults() and iperf_reset_test().

[dxbjavid]

makes sense, keeping the buffer at COOKIE_SIZE + 1 with a zeroed guard byte is nicer since it preserves all 37 received bytes rather than truncating the effective cookie to 36. it also lines up with make_cookie(), which already assumes a COOKIE_SIZE + 1 buffer.

i've reopened, rebased onto master and dropped the earlier change. iperf.h now declares cookie[COOKIE_SIZE + 1], and both memsets in iperf_defaults() and iperf_reset_test() zero COOKIE_SIZE + 1 bytes so the trailing guard is always NUL. built cleanly and a quick loopback run still exchanges the cookie fine.