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

diff --git a/CHANGES b/CHANGES
index 4c512bc..1b55dc4 100644 (file)
--- 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 4e864f5..1d87294 100644 (file)
--- 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 141c575..20e5fd5 100644 (file)
--- 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;