Coverity reported buffer overrun error in "winebuild: Add support for 16-bit exe modules."

Dan Kegel dank at kegel.com
Sat Jun 28 09:17:42 CDT 2008


Coverity complains that
http://source.winehq.org/git/wine.git/?a=commitdiff;h=0c214a7091af8efe39ffdaea7fe9e2de4d8006ba
introduced a buffer overrun in winebuild.  It looks like
somebody forgot to dynamically grow an array?

Here's the report.  Can somebody familiar with the code (or
with a little time on their hands) have a look?
Thanks...
- Dan

CID: 682
Checker: OVERRUN_DYNAMIC (help)
File: base/src/wine/tools/winebuild/spec16.c
Function: BuildSpec16File
Description: Overrun of dynamic array "(spec)->ordinals" of size 4 at
position 4 with index variable "(i * 4)"

524  	/*******************************************************************
525  	 *         BuildSpec16File
526  	 *
527  	 * Build a Win16 assembly file from a spec file.
528  	 */
529  	void BuildSpec16File( DLLSPEC *spec )
530  	{
531  	    ORDDEF **typelist;
532  	    ORDDEF *entry_point = NULL;
533  	    int i, j, nb_funcs;
534  	    char header_name[256];
535  	
536  	    /* File header */
537  	
538  	    output_standard_file_header();
539  	
540  	    if (!spec->file_name)
541  	    {
542  	        char *p;
543  	        spec->file_name = xstrdup( output_file_name );
544  	        if ((p = strrchr( spec->file_name, '.' ))) *p = 0;
545  	    }
546  	    if (!spec->dll_name)  /* set default name from file name */
547  	    {
548  	        char *p;
549  	        spec->dll_name = xstrdup( spec->file_name );
550  	        if ((p = strrchr( spec->dll_name, '.' ))) *p = 0;
551  	    }
552  	
553  	    /* store the main entry point as ordinal 0 */
554  	
555  	    if (!spec->ordinals)
556  	    {

Event buffer_alloc: Called allocating function "xmalloc" which
allocated 4 bytes dictated by parameter 0 [model]
Event var_assign: Assigned "(spec)->ordinals" to storage allocated by "xmalloc"
Also see events: [var_assign][overrun-local]

557  	        spec->ordinals = xmalloc( sizeof(spec->ordinals[0]) );
558  	        spec->ordinals[0] = NULL;
559  	    }

At conditional (1): "(spec)->init_func != 0" taking true path
At conditional (2): "(spec)->characteristics & 8192 == 0" taking true path

560  	    if (spec->init_func && !(spec->characteristics & IMAGE_FILE_DLL))
561  	    {
562  	        entry_point = xmalloc( sizeof(*entry_point) );
563  	        entry_point->type = TYPE_PASCAL;
564  	        entry_point->ordinal = 0;
565  	        entry_point->lineno = 0;
566  	        entry_point->flags = FLAG_REGISTER;
567  	        entry_point->name = NULL;
568  	        entry_point->link_name = xstrdup( spec->init_func );
569  	        entry_point->export_name = NULL;
570  	        entry_point->u.func.arg_types[0] = 0;

At conditional (3): "*((spec)->ordinals + 0) == 0" taking true path

571  	        assert( !spec->ordinals[0] );
572  	        spec->ordinals[0] = entry_point;
573  	    }
574  	
575  	    /* Build sorted list of all argument types, without duplicates */
576  	
577  	    typelist = xmalloc( (spec->limit + 1) * sizeof(*typelist) );
578  	

At conditional (4): "i <= (spec)->limit" taking true path
At conditional (6): "i <= (spec)->limit" taking true path
At conditional (8): "i <= (spec)->limit" taking true path
At conditional (10): "i <= (spec)->limit" taking true path
At conditional (13): "i <= (spec)->limit" taking true path
At conditional (16): "i <= (spec)->limit" taking false path

579  	    for (i = nb_funcs = 0; i <= spec->limit; i++)
580  	    {
581  	        ORDDEF *odp = spec->ordinals[i];

At conditional (5): "odp == 0" taking true path
At conditional (7): "odp == 0" taking true path
At conditional (9): "odp == 0" taking true path
At conditional (11): "odp == 0" taking false path
At conditional (14): "odp == 0" taking false path

582  	        if (!odp) continue;

At conditional (12): "is_function != 0" taking false path
At conditional (15): "is_function != 0" taking true path

583  	        if (is_function( odp )) typelist[nb_funcs++] = odp;
584  	    }
585  	
586  	    nb_funcs = sort_func_list( typelist, nb_funcs,
callfrom16_type_compare );
587  	
588  	    /* Output the module structure */
589  	
590  	    sprintf( header_name, "__wine_spec_%s_dos_header",
make_c_identifier(spec->dll_name) );
591  	    output( "\n/* module data */\n\n" );
592  	    output( "\t.data\n" );
593  	    output( "\t.align %d\n", get_alignment(4) );
594  	    output( "%s:\n", header_name );
595  	    output( "\t%s 0x%04x\n", get_asm_short_keyword(),
          /* e_magic */
596  	             IMAGE_DOS_SIGNATURE );
597  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_cblp */
598  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_cp */
599  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_crlc */
600  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_cparhdr */
601  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_minalloc */
602  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_maxalloc */
603  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_ss */
604  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_sp */
605  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_csum */
606  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_ip */
607  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_cs */
608  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_lfarlc */
609  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_ovno */
610  	    output( "\t%s 0,0,0,0\n", get_asm_short_keyword() );
          /* e_res */
611  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_oemid */
612  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* e_oeminfo */
613  	    output( "\t%s 0,0,0,0,0,0,0,0,0,0\n",
get_asm_short_keyword() );       /* e_res2 */
614  	    output( "\t.long .L__wine_spec_ne_header-%s\n", header_name
);         /* e_lfanew */
615  	
616  	    output( ".L__wine_spec_ne_header:\n" );
617  	    output( "\t%s 0x%04x\n", get_asm_short_keyword(),
          /* ne_magic */
618  	             IMAGE_OS2_SIGNATURE );
619  	    output( "\t.byte 0\n" );
          /* ne_ver */
620  	    output( "\t.byte 0\n" );
          /* ne_rev */
621  	    output( "\t%s
.L__wine_spec_ne_enttab-.L__wine_spec_ne_header\n",      /* ne_enttab
*/
622  	             get_asm_short_keyword() );
623  	    output( "\t%s
.L__wine_spec_ne_enttab_end-.L__wine_spec_ne_enttab\n",  /*
ne_cbenttab */
624  	             get_asm_short_keyword() );
625  	    output( "\t.long 0\n" );
          /* ne_crc */

At conditional (17): "(spec)->characteristics & 8192 != 0" taking false path

626  	    output( "\t%s 0x%04x\n", get_asm_short_keyword(),
          /* ne_flags */
627  	             NE_FFLAGS_SINGLEDATA |
628  	             ((spec->characteristics & IMAGE_FILE_DLL) ?
NE_FFLAGS_LIBMODULE : 0) );
629  	    output( "\t%s 2\n", get_asm_short_keyword() );
          /* ne_autodata */
630  	    output( "\t%s %u\n", get_asm_short_keyword(),
spec->heap_size );       /* ne_heap */
631  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* ne_stack */

At conditional (18): "entry_point == 0" taking false path

632  	    if (!entry_point) output( "\t.long 0\n" );
          /* ne_csip */
633  	    else output( "\t%s .L__wine_%s_0-.L__wine_spec_code_segment,1\n",
634  	                 get_asm_short_keyword(),
make_c_identifier(spec->dll_name) );
635  	    output( "\t%s 0,2\n", get_asm_short_keyword() );
          /* ne_sssp */
636  	    output( "\t%s 2\n", get_asm_short_keyword() );
          /* ne_cseg */
637  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* ne_cmod */
638  	    output( "\t%s 0\n", get_asm_short_keyword() );
          /* ne_cbnrestab */
639  	    output( "\t%s
.L__wine_spec_ne_segtab-.L__wine_spec_ne_header\n",      /* ne_segtab
*/
640  	             get_asm_short_keyword() );
641  	    output( "\t%s
.L__wine_spec_ne_rsrctab-.L__wine_spec_ne_header\n",     /* ne_rsrctab
*/
642  	             get_asm_short_keyword() );
643  	    output( "\t%s
.L__wine_spec_ne_restab-.L__wine_spec_ne_header\n",      /* ne_restab
*/
644  	             get_asm_short_keyword() );
645  	    output( "\t%s
.L__wine_spec_ne_modtab-.L__wine_spec_ne_header\n",      /* ne_modtab
*/
646  	             get_asm_short_keyword() );
647  	    output( "\t%s
.L__wine_spec_ne_imptab-.L__wine_spec_ne_header\n",      /* ne_imptab
*/
648  	             get_asm_short_keyword() );
649  	    output( "\t.long 0\n" );
/* ne_nrestab */
650  	    output( "\t%s 0\n", get_asm_short_keyword() );
/* ne_cmovent */
651  	    output( "\t%s 0\n", get_asm_short_keyword() );
/* ne_align */
652  	    output( "\t%s 0\n", get_asm_short_keyword() );
/* ne_cres */
653  	    output( "\t.byte 0x%02x\n", NE_OSFLAGS_WINDOWS );
/* ne_exetyp */
654  	    output( "\t.byte 0x%02x\n", NE_AFLAGS_FASTLOAD );
/* ne_flagsothers */
655  	    output( "\t%s 0\n", get_asm_short_keyword() );
/* ne_pretthunks */
656  	    output( "\t%s 0\n", get_asm_short_keyword() );
/* ne_psegrefbytes */
657  	    output( "\t%s 0\n", get_asm_short_keyword() );
/* ne_swaparea */
658  	    output( "\t%s 0\n", get_asm_short_keyword() );
/* ne_expver */
659  	
660  	    /* segment table */
661  	
662  	    output( "\n.L__wine_spec_ne_segtab:\n" );
663  	
664  	    /* code segment entry */
665  	
666  	    output( "\t%s .L__wine_spec_code_segment-%s\n",  /* filepos */
667  	             get_asm_short_keyword(), header_name );
668  	    output( "\t%s
.L__wine_spec_code_segment_end-.L__wine_spec_code_segment\n", /* size
*/
669  	             get_asm_short_keyword() );
670  	    output( "\t%s 0x%04x\n", get_asm_short_keyword(),
NE_SEGFLAGS_32BIT );      /* flags */
671  	    output( "\t%s
.L__wine_spec_code_segment_end-.L__wine_spec_code_segment\n", /*
minsize */
672  	             get_asm_short_keyword() );
673  	
674  	    /* data segment entry */
675  	
676  	    output( "\t%s .L__wine_spec_data_segment-%s\n",  /* filepos */
677  	             get_asm_short_keyword(), header_name );
678  	    output( "\t%s
.L__wine_spec_data_segment_end-.L__wine_spec_data_segment\n", /* size
*/
679  	             get_asm_short_keyword() );
680  	    output( "\t%s 0x%04x\n", get_asm_short_keyword(),
NE_SEGFLAGS_DATA );      /* flags */
681  	    output( "\t%s
.L__wine_spec_data_segment_end-.L__wine_spec_data_segment\n", /*
minsize */
682  	             get_asm_short_keyword() );
683  	
684  	    /* resource directory */
685  	
686  	    output_res16_directory( spec, header_name );
687  	
688  	    /* resident names table */
689  	
690  	    output( "\n\t.align %d\n", get_alignment(2) );
691  	    output( ".L__wine_spec_ne_restab:\n" );
692  	    output_resident_name( spec->dll_name, 0 );

At conditional (19): "i <= (spec)->limit" taking true path

693  	    for (i = 1; i <= spec->limit; i++)
694  	    {

Event overrun-local: Overrun of dynamic array "(spec)->ordinals" of
size 4 at position 4 with index variable "(i * 4)"
Also see events: [buffer_alloc][var_assign]

695  	        ORDDEF *odp = spec->ordinals[i];



More information about the wine-devel mailing list