[Bug 1001522] New: Array index out of bounds in tftp_server.c

[bugzilla-daemon at bugs dot ecos dot sourceware dot org]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001522

           Summary: Array index out of bounds in tftp_server.c
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: minor
          Priority: low
         Component: TCP/IP
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: grant.b.edwards@gmail.com
                CC: ecos-bugs@ecos.sourceware.org
             Class: Advice Request


There appears to be an array index out-of-bounds problem in
tftp_server.c at line 691.  At lines 661-666 there is a for loop that
leaves i with the value CYGNUM_NET_MAX_INET_PROTOS.  Then at line 691,
'i' is used as a subscript in the experess 'server->s[i]'.  The array
's' contains CYGNUM_NET_MAX_INET_PROTOS elements, so the max legal
subscript is CYGNUM_NET_MAX_INET_PROTOS-1, but i is
CYGNUM_NET_MAX_INET_PROTOS at that point.

I have no clue what's going on in this particular code.  The use of i
as a loop index inside an outer loop that also uses i as the loop
index seems like a mistake.  I suspect that the loop at lines 661-666
should not be using i as the loop index.


   647          for (i=0; i < CYGNUM_NET_MAX_INET_PROTOS; i++) {
   648            if (server->s[i] && FD_ISSET(server->s[i],&readfds)) {
   649              recv_len = sizeof(data);
   650              from_len = sizeof(from_addr);
   651              data_len = recvfrom(server->s[i], hdr, recv_len, 0,
   652                                  &from_addr, &from_len);
   653              if ( data_len < 0) {
   654                diag_printf("TFTPD [%x]: can't read request\n", p);
   655              } else {
   656  #ifdef CYGSEM_NET_TFTPD_MULTITHREADED
   657                // Close the socket and post on the semaphore some
   658                // another thread can start listening for requests. This
   659                // is not quite right. select could of returned with more
than
   660                // one socket with data to read. Here we only deal with
one of them
   661                for (i=0; i < CYGNUM_NET_MAX_INET_PROTOS; i++) {
   662                  if (server->s[i]) {
   663                    close (server->s[i]);
   664                    server->s[i] = 0;
   665                  }
   666                }
   667                sem_post(server->port);
   668  #endif
   669  #ifndef CYGPKG_NET_TESTS_USE_RT_TEST_HARNESS
   670                getnameinfo(&from_addr,sizeof(from_addr), name,
sizeof(name),0,0,0);
   671                diag_printf("TFTPD [%x]: received %x from %s\n", p,
   672                            ntohs(hdr->th_opcode), name);
   673  #endif
   674                switch (ntohs(hdr->th_opcode)) {
   675                case WRQ:
   676                  tftpd_write_file(server, hdr, &from_addr, from_len);
   677                  break;
   678                case RRQ:
   679                  tftpd_read_file(server, hdr, &from_addr, from_len);
   680                  break;
   681                case ACK:
   682                case DATA:
   683                case ERROR:
   684                  // Ignore
   685                  break;
   686                default:
   687                  getnameinfo(&from_addr,sizeof(from_addr), name,
sizeof(name),0,0,0);
   688                  diag_printf("TFTPD [%x]: bogus request %x from %s\n",
p,
   689                              ntohs(hdr->th_opcode),
   690                              name);
   691                 
tftpd_send_error(server->s[i],hdr,TFTP_EBADOP,&from_addr,from_len);
   692                }
   693

It seems clear that the loop at 661-666 should not be using i.

However, I'm reluctant to change it without more discussion since I
don't use tftp and don't know how such a "fix" would change the code's
behavior.  Somebody may be depending somehow on the existing behavior
-- it wouldn't be the first time that fixing a bug broke something.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.