#45 -log causes blat.exe to crash on XP and 2003 Server

open
nobody
None
5
2007-08-30
2007-08-30
Anonymous
No

When using blat.exe Version 2.6.2 with the Parameter -log it crashes.

Example:
blat.exe test.txt -t mail@example.org -s "Test Blat" -log C:\temp\blat.log

Returns -1073741819 (%ERRORLEVEL%)

Windows XP reports an error in application: Instruction in "0x77c16fa3" points to "0x00324000". Instruction "read" could not be executed (raw translation)

Version 2.6.1 seems to work well.

Under Windows 2003 Server it (2.6.2) works as long as it is not run as scheduled task. Then blat.exe produces the following error:

Faulting application blat.exe, version 2.6.2.0, faulting module msvcrt.dll, version 7.0.3790.1830, fault address 0x00037e23.

Discussion

  • Nobody/Anonymous

    Logged In: NO

    Same problem exist on Windows 2000 Server with Version 2.6.2

     
  • Chip

    Chip - 2008-03-15

    Logged In: YES
    user_id=800692
    Originator: NO

    Ah, the answer is in your last line, the failure is caused by MSVCRT.DLL. Here is the file I use with my WinXP SP2: http://home.att.net/~chip.programmer/blat/msvcrt.dll

     
  • Nobody/Anonymous

    OPTIONS.CPP 1712

    memcpy(bodyFilename, argv[this_arg+1], sizeof(bodyFilename)-1 );

    OPTIONS.CPP 2464
    memcpy( logFile, argv[this_arg+1], sizeof(logFile) - 1 );

    these lines are cause of crash.
    the source size is not taken into account.
    so sometimes we have crash when using -log and <filename>.
    they should be changed to
    memset(bodyFilename, 0, sizeof(bodyFilename));
    memcpy(bodyFilename, argv[this_arg+1], min(sizeof(bodyFilename)-1, strlen(bodyFilename)) );

    and

    memset(logFile, 0, sizeof(logFile));
    memcpy( logFile, argv[this_arg+1], min(sizeof(logFile) - 1, strlen(argv[this_arg+1])));
    respectively.

     
  • Nobody/Anonymous

    memcpy(bodyFilename, argv[this_arg+1], min(sizeof(bodyFilename)-1, strlen(argv[this_arg+1])) );
    of course

     
  • Chip

    Chip - 2008-11-06

    Let's look at the two routines to see what they are doing, and their associated character arrays:

    extern char bodyFilename[_MAX_PATH];
    static char logFile[_MAX_PATH];

    static int checkBodyFile ( int argc, char ** argv, int this_arg, int startargv )
    {
    argc = argc; // For eliminating compiler warnings.
    startargv = startargv;

    memcpy(bodyFilename, argv[this_arg+1], sizeof(bodyFilename)-1 );
    bodyFilename[sizeof(bodyFilename)-1] = 0;

    return(1);
    }

    static int checkLogMessages ( int argc, char ** argv, int this_arg, int startargv )
    {
    argc = argc; // For eliminating compiler warnings.
    startargv = startargv;

    if ( logOut )
    fclose( logOut );

    memcpy( logFile, argv[this_arg+1], sizeof(logFile) - 1 );
    logFile[sizeof(logFile) - 1] = 0;

    if ( clearLogFirst )
    logOut = fopen(logFile, "w");
    else
    logOut = fopen(logFile, "a");

    // if all goes well the file is closed normally
    // if we return before closing the library SHOULD
    // close it for us..

    return(1);
    }

    As you can see, these two character arrays have been defined to be of _MAX_PATH length.

    If we look at the actual assembly code that was generated, we see the follow for checkBodyFile():

    0000 ?checkBodyFile@@YAHHPAPADHH@Z:
    0000 8B 44 24 0C mov eax,dword ptr 0xc[esp]
    0004 8B 4C 24 08 mov ecx,dword ptr 0x8[esp]
    0008 68 03 01 00 00 push 0x00000103
    000D FF 74 81 04 push dword ptr 0x4[ecx+eax*4]
    0011 68 00 00 00 00 push ?bodyFilename@@3PADA
    0016 E8 00 00 00 00 call j^_memcpy
    001B 33 C0 xor eax,eax
    001D 83 C4 0C add esp,0x0000000c
    0020 C6 05 03 01 00 00 00 mov byte ptr ?bodyFilename@@3PADA+0x103,0x00
    0027 40 inc eax
    0028 C3 ret

    According to the memcpy() statement, there should be _MAX_PATH-1 *bytes* copied from argv[this_arg+1] into bodyFilename[], as represented by "sizeof(bodyFilename)-1". Since _MAX_PATH is defined as 260 by Windows, and 260 decimal is the same as 0x00000104 in hexadecimal, _MAX_PATH-1 is then represented as 0x00000103 that can be seen in the assembly code above. This means that your comments about the size of the target (bodyFilename) is inaccurate. The truth is that the size of the receiving array *is* accounted for.

    When we view the assembly code generated for checkLogMessages(), we see similar code:

    0000 ?checkLogMessages@@YAHHPAPADHH@Z:
    0000 A1 00 00 00 00 mov eax,dword ptr ?logOut@@3PAU_iobuf@@A
    0005 85 C0 test eax,eax
    0007 74 08 je X$208
    0009 50 push eax
    000A FF 15 00 00 00 00 call dword ptr __imp__fclose
    0010 59 pop ecx
    0011 X$208:
    0011 8B 44 24 0C mov eax,dword ptr 0xc[esp]
    0015 8B 4C 24 08 mov ecx,dword ptr 0x8[esp]
    0019 56 push esi
    001A 68 03 01 00 00 push 0x00000103
    001F FF 74 81 04 push dword ptr 0x4[ecx+eax*4]
    0023 BE 00 00 00 00 mov esi,_logFile
    0028 56 push esi
    0029 E8 00 00 00 00 call j^_memcpy
    002E 83 C4 0C add esp,0x0000000c
    0031 80 3D 00 00 00 00 00 cmp byte ptr ?clearLogFirst@@3DA,0x00
    0038 C6 05 03 01 00 00 00 mov byte ptr _logFile+0x103,0x00
    003F 74 07 je X$209
    0041 68 00 00 00 00 push ??_C@_01NOFIACDB@w?$AA@
    0046 EB 05 jmp X$210
    0048 X$209:
    0048 68 00 00 00 00 push ??_C@_01MCMALHOG@a?$AA@
    004D X$210:
    004D 56 push esi
    004E FF 15 00 00 00 00 call dword ptr __imp__fopen
    0054 59 pop ecx
    0055 A3 00 00 00 00 mov dword ptr ?logOut@@3PAU_iobuf@@A,eax
    005A 59 pop ecx
    005B 33 C0 xor eax,eax
    005D 40 inc eax
    005E 5E pop esi
    005F C3 ret

    In both of these routines, what I do not have is any code to validate argv[this_arg+1] to be non-zero or that the first byte of that argument is non-zero. I will create version 2.7.1 in a moment with this change, and post it to my download site at http://home.att.net/~chip.programmer/blat/v2.71/blat.exe

    Chip

     
  • Nobody/Anonymous

    No.
    My comment is about source size, not target.

    "According to the memcpy() statement, there should be _MAX_PATH-1 *bytes*
    copied from argv[this_arg+1] into bodyFilename[], as represented by
    "sizeof(bodyFilename)-1"."
    right. but argv[this_arg+1] is not _MAX_PATH-1 in length. so we read some bytes after end of argv[this_arg+1]. and when argv[this_arg+1] is close to end of page then we get error "Instruction "read" could not be executed".
    may be you should use something like
    strncpy(bodyFilename, argv[this_arg+1], sizeof(bodyFilename)-1 );
    .

    Dima

     
  • Chip

    Chip - 2008-12-05

    The original message made it seem like you were referring to the target array, not the source array. I will change the memcpy() to strncpy() based on the argument that your source array might be closer to the end of a Windows page than the size of my target array. Thank you for the added clarification in your first line. :)

    Chip

     

Log in to post a comment.