Commit 0a29e244 authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

TFTP: block id wrap bug fix 

In a normal expression, doing [unsigned short] + 1 will not wrap
at 16 bits so the comparisons and outputs were done wrong. I
added a macro do make sure it gets done right.

Douglas Kilpatrick filed bug report #3004787 about it:
http://curl.haxx.se/bug/view.cgi?id=3004787
parent 70033510

CHANGES

+6 −0
Original line number Diff line number Diff line
@@ -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.

RELEASE-NOTES

+2 −1
Original line number Diff line number Diff line
@@ -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)

lib/tftp.c

+11 −7
Original line number Diff line number Diff line
@@ -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,7 +1106,7 @@ 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);