Coverity reported buffer overrun error in "winebuild: Add support for 16-bit exe modules."
Marcus Meissner
marcus at jet.franken.de
Sat Jun 28 09:37:37 CDT 2008
On Sat, Jun 28, 2008 at 07:17:42AM -0700, Dan Kegel wrote:
> 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
I thought it to be a mixup between EXE and DLL generation there, but
I did not understand it either. (that the EXE generator has the ordinals[1]
array)
Ciao, Marcus
> 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