diff --git a/CHANGES b/CHANGES index 4c512bcd2..1b55dc432 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,12 @@ Changelog +Daniel Stenberg (21 May 2010) +- Douglas Kilpatrick filed bug report #3004787 and pointed out that the TFTP + code didn't handle block id wraps correctly. His suggested fix inspired the + fix I committed. + + (http://curl.haxx.se/bug/view.cgi?id=3004787) Daniel Stenberg (20 May 2010) - Tanguy Fautre brought a fix to allow curl to build with Microsoft VC10. diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 4e864f5d9..1d872945d 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -31,6 +31,7 @@ This release includes the following bugfixes: o ignore response-body on redirect even if compressed o OpenSSL handshake state-machine for multi interface o TFTP timeout option sent correctly + o TFTP block id wrap This release includes the following known bugs: @@ -41,6 +42,6 @@ advice from friends like these: Rainer Canavan, Paul Howarth, Jerome Vouillon, Ruslan Gazizov, Yang Tse, Kamil Dudka, Alex Bligh, Ben Greear, Hoi-Ho Chan, Howard Chu, Dirk Manske, - Pavel Raiskup, John-Mark Bell, Eric Mertens, Tor Arntsen + Pavel Raiskup, John-Mark Bell, Eric Mertens, Tor Arntsen, Douglas Kilpatrick Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/tftp.c b/lib/tftp.c index 141c57570..20e5fd5da 100644 --- a/lib/tftp.c +++ b/lib/tftp.c @@ -572,6 +572,10 @@ static CURLcode tftp_send_first(tftp_state_data_t *state, tftp_event_t event) return res; } +/* the next blocknum is x + 1 but it needs to wrap at an unsigned 16bit + boundary */ +#define NEXT_BLOCKNUM(x) (((x)+1)&0xffff) + /********************************************************** * * tftp_rx @@ -590,14 +594,14 @@ static CURLcode tftp_rx(tftp_state_data_t *state, tftp_event_t event) case TFTP_EVENT_DATA: /* Is this the block we expect? */ rblock = getrpacketblock(&state->rpacket); - if((state->block+1) != rblock) { + if(NEXT_BLOCKNUM(state->block) != rblock) { /* No, log it, up the retry count and fail if over the limit */ infof(data, "Received unexpected DATA packet block %d\n", rblock); state->retries++; if(state->retries>state->retry_max) { failf(data, "tftp_rx: giving up waiting for block %d", - state->block+1); + NEXT_BLOCKNUM(state->block)); return CURLE_TFTP_ILLEGAL; } } @@ -650,7 +654,7 @@ static CURLcode tftp_rx(tftp_state_data_t *state, tftp_event_t event) state->retries++; infof(data, "Timeout waiting for block %d ACK. Retries = %d\n", - state->block+1, state->retries); + NEXT_BLOCKNUM(state->block), state->retries); if(state->retries > state->retry_max) { state->error = TFTP_ERR_TIMEOUT; state->state = TFTP_STATE_FIN; @@ -773,7 +777,7 @@ static CURLcode tftp_tx(tftp_state_data_t *state, tftp_event_t event) /* Increment the retry counter and log the timeout */ state->retries++; infof(data, "Timeout waiting for block %d ACK. " - " Retries = %d\n", state->block+1, state->retries); + " Retries = %d\n", NEXT_BLOCKNUM(state->block), state->retries); /* Decide if we've had enough */ if(state->retries > state->retry_max) { state->error = TFTP_ERR_TIMEOUT; @@ -1102,10 +1106,10 @@ static CURLcode tftp_receive_packet(struct connectdata *conn) case TFTP_EVENT_DATA: /* Don't pass to the client empty or retransmitted packets */ if(state->rbytes > 4 && - ((state->block+1) == getrpacketblock(&state->rpacket))) { + (NEXT_BLOCKNUM(state->block) == getrpacketblock(&state->rpacket))) { result = Curl_client_write(conn, CLIENTWRITE_BODY, - (char *)state->rpacket.data+4, - state->rbytes-4); + (char *)state->rpacket.data+4, + state->rbytes-4); if(result) { tftp_state_machine(state, TFTP_EVENT_ERROR); return result;