Re: array index bug in tftp_server.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

On 2012-03-01 16:38, Grant Edwards wrote:
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

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 {
    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
    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	              }

It looks like the loop variable 'i' is reused (incorrectly)
inside the #ifdef CYGSEM_NET_TFTPD_MULTITHREADED starting
on line 656.  Change that to be a different variable and it
should fix it.

Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world

Before posting, please read the FAQ:
and search the list archive:

[Linux Embedded]     [U-Boot V2]     [Linux Kernel]     [Linux MIPS]     [Linux ARM]     [Linux for the Blind]     [Linux Resources]     [Photo]     [Yosemite]     [ISDN Cause Codes]     [ECOS Home]

Add to Google Powered by Linux