This is the mail archive of the ecos-discuss@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: array index bug in tftp_server.c


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
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 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: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]