Overflow issue with _srcport in Ethernet Library

[GoogleCodeExporter]

In Arduino-0014 in Client.cpp from the Ethernet library there is an overflow 
issue with the _srcport 
in the connect() function. The _srcport is incremented on every connect without 
any limits. Since it's 
a uint16_t the assumption is that it would just overflow back to zero without 
any problems.

This would normally be okay, but the code then adds 1024 to the _srcport during 
the socket() call. 
This is correct to keep the port out of the privileged range. The problem is 
that once the _srcport 
exceeds 64111 the addition of 1024 will make the _srcport overflow back into 
the privileged port 
range.

There needs to be a check added after the _srcport++ to reset the value to 0 if 
it is > 54511.

Original issue reported on code.google.com by etrace...@gmail.com on 25 Mar 2009 at 12:08

[GoogleCodeExporter]

Can you write up a patch to fix this and send it to the developers mailing 
list?  http://arduino.cc/mailman/listinfo/developers_arduino.cc

Original comment by dmel...@gmail.com on 27 Mar 2009 at 3:37

• Added labels: Priority-Low
• Removed labels: Priority-Medium

[GoogleCodeExporter]

I tried registering for the developers mailing list yesterday but still have 
not received the confirmation email. 
Maybe there's something wrong with the mailman system?

The only change is a simple test to reset _srcport if it's over 64511 (note 
that I made a type in the original post 
and indicated 54511).

In Client.cpp:

Original connect() function:
***************************************************
uint8_t Client::connect() {
  _sock = 255;

  for (int i = 0; i < MAX_SOCK_NUM; i++) {
    if (getSn_SR(i) == SOCK_CLOSED) {
      _sock = i;
    }
  }

  if (_sock == 255)
    return 0;

  _srcport++;

  socket(_sock, Sn_MR_TCP, 1024 + _srcport, 0);

  if (!::connect(_sock, _ip, _port))
    return 0;

  while (status() != SOCK_ESTABLISHED) {
    if (status() == SOCK_CLOSED)
      return 0;
  }

  return 1;
}
***************************************************

Revised connect() function:
***************************************************
uint8_t Client::connect() {
  _sock = 255;

  for (int i = 0; i < MAX_SOCK_NUM; i++) {
    if (getSn_SR(i) == SOCK_CLOSED) {
      _sock = i;
    }
  }

  if (_sock == 255)
    return 0;

  _srcport++;

  if (_srcport > 64511)
    _srcport = 0;

  socket(_sock, Sn_MR_TCP, 1024 + _srcport, 0);

  if (!::connect(_sock, _ip, _port))
    return 0;

  while (status() != SOCK_ESTABLISHED) {
    if (status() == SOCK_CLOSED)
      return 0;
  }

  return 1;
}
***************************************************

The only change being the addition of:
***************************************************
 if (_srcport > 64511)
    _srcport = 0;
***************************************************

Original comment by etrace...@gmail.com on 28 Mar 2009 at 4:38

[GoogleCodeExporter]

This should be fixed in 0016.

Original comment by dmel...@gmail.com on 5 Jun 2009 at 12:34

• Changed state: Fixed