masm32 GetCl is bugged

[TouEnMasm]

The function don't verify the size of the buffer before write in it.
This result in a crash if the command line exceed 192 bytes (little command line).
Here is the corrected version.
File is named getcl.asm

Code Select Expand


; #########################################################################

      .386                      ; force 32 bit code
      .model flat, stdcall      ; memory model & calling convention
      option casemap :none      ; case sensitive

      include \masm32\include\kernel32.inc

      GetCL PROTO :DWORD,:DWORD

    .code

; #########################################################################

GetCL proc ArgNum:DWORD, ItemBuffer:DWORD

  ; -------------------------------------------------
  ; arguments returned in "ItemBuffer"
  ;
  ; arg 0 = program name
  ; arg 1 = 1st arg
  ; arg 2 = 2nd arg etc....
  ; -------------------------------------------------
  ; Return values in eax
  ;
  ; 1 = successful operation
  ; 2 = no argument exists at specified arg number
  ; 3 = non matching quotation marks
  ; 4 = empty quotation marks
  ; 5 = buffer too small
  ; -------------------------------------------------

    LOCAL lpCmdLine      :DWORD
    LOCAL cmdBuffer[600h] :BYTE   ;changed
    LOCAL tmpBuffer[600h] :BYTE   ;changed

    push esi
    push edi

    invoke GetCommandLine
    mov lpCmdLine, eax        ; address command line

  ; -------------------------------------------------
  ; count quotation marks to see if pairs are matched
  ; -------------------------------------------------
    xor ecx, ecx            ; zero ecx & use as counter
    mov esi, lpCmdLine
   
    @@:
      lodsb
      cmp al, 0
      je @F
      cmp al, 34            ; [ " ] character
      jne @B
      inc ecx               ; increment counter
      jmp @B
    @@:

    push ecx                ; save count

    shr ecx, 1              ; integer divide ecx by 2
    shl ecx, 1              ; multiply ecx by 2 to get dividend

    pop eax                 ; put count in eax
    cmp eax, ecx            ; check if they are the same
    je @F
      pop edi
      pop esi
      mov eax, 3            ; return 3 in eax = non matching quotation marks
      ret
    @@:

  ; ------------------------
  ; replace tabs with spaces
  ; ------------------------
    mov esi, lpCmdLine
    ;-------------- changed ------------------
    invoke lstrlen,esi
    .if eax > 600h -1
mov eax,5 ;buffer too small
      pop edi
      pop esi
      ret           
    .endif
    ;-----------------------------------------
    lea edi, cmdBuffer

    @@:
      lodsb
      cmp al, 0
      je rtOut
      cmp al, 9     ; tab
      jne rtIn
      mov al, 32
    rtIn:
      stosb
      jmp @B
    rtOut:
      stosb         ; write last byte

  ; -----------------------------------------------------------
  ; substitute spaces in quoted text with replacement character
  ; -----------------------------------------------------------
    lea eax, cmdBuffer
    mov esi, eax
    mov edi, eax

    subSt:
      lodsb
      cmp al, 0
      jne @F
      jmp subOut
    @@:
      cmp al, 34
      jne subNxt
      stosb
      jmp subSl     ; goto subloop
    subNxt:
      stosb
      jmp subSt

    subSl:
      lodsb
      cmp al, 32    ; space
      jne @F
        mov al, 254 ; substitute character
      @@:
      cmp al, 34
      jne @F
        stosb
        jmp subSt
      @@:
      stosb
      jmp subSl

    subOut:
      stosb         ; write last byte

  ; ----------------------------------------------------
  ; the following code determines the correct arg number
  ; and writes the arg into the destination buffer
  ; ----------------------------------------------------
    lea eax, cmdBuffer
    mov esi, eax
    lea edi, tmpBuffer

    mov ecx, 0          ; use ecx as counter

  ; ---------------------------
  ; strip leading spaces if any
  ; ---------------------------
    @@:
      lodsb
      cmp al, 32
      je @B

    l2St:
      cmp ecx, ArgNum     ; the number of the required cmdline arg
      je clSubLp2
      lodsb
      cmp al, 0
      je cl2Out
      cmp al, 32
      jne cl2Ovr           ; if not space

    @@:
      lodsb
      cmp al, 32          ; catch consecutive spaces
      je @B

      inc ecx             ; increment arg count
      cmp al, 0
      je cl2Out

    cl2Ovr:
      jmp l2St

    clSubLp2:
      stosb
    @@:
      lodsb
      cmp al, 32
      je cl2Out
      cmp al, 0
      je cl2Out
      stosb
      jmp @B

    cl2Out:
      mov al, 0
      stosb

  ; ------------------------------
  ; exit if arg number not reached
  ; ------------------------------
    .if ecx < ArgNum
      mov edi, ItemBuffer
      mov al, 0
      stosb
      mov eax, 2  ; return value of 2 means arg did not exist
      pop edi
      pop esi
      ret
    .endif

  ; -------------------------------------------------------------
  ; remove quotation marks and replace the substitution character
  ; -------------------------------------------------------------
    lea eax, tmpBuffer
    mov esi, eax
    mov edi, ItemBuffer

    rqStart:
      lodsb
      cmp al, 0
      je rqOut
      cmp al, 34    ; dont write [ " ] mark
      je rqStart
      cmp al, 254
      jne @F
      mov al, 32    ; substitute space
    @@:
      stosb
      jmp rqStart

  rqOut:
      stosb         ; write zero terminator

  ; ------------------
  ; handle empty quote
  ; ------------------
    mov esi, ItemBuffer
    lodsb
    cmp al, 0
    jne @F
    pop edi
    pop esi
    mov eax, 4  ; return value for empty quote
    ret
  @@:

    mov eax, 1  ; return value success

    pop edi
    pop esi

    ret

GetCL endp

; #########################################################################

end



[jj2007]

This result in a crash if the command line exceed 192 bytes (little command line).

Typically, apps get less than 128 bytes passed. How do you produce that crash?

[TouEnMasm]

in the explorer,he could pass any number of files you want.

[dedndave]

127 bytes is the limit for 16-bit code
i think the command line can be 32768, as i recall

[MichaelW]

Under Windows XP SP3, and using the ANSI version of GetCommandLine, by selecting a number of files in Explorer, then copying the selection and using paste to drop it on an app built from the source below, I can reach a 2081-byte command line. But at some point beyond this, and I didn't take the time to determine exactly where, I get an error dialog with:

"Windows cannot access the specified device, path, or file. You may not have the appropriate permissions to access the item".

Code Select Expand


;==============================================================================
    include \masm32\include\masm32rt.inc
;==============================================================================
    .data
    .code
;==============================================================================
start:
;==============================================================================
    invoke GetCommandLine
    mov ebx, eax
    invoke szLen, ebx
    printf("%d\n\n",eax)
    printf("%s\n\n",ebx)
    inkey
    exit
;==============================================================================
end start


Edit:

The strange error message was apparently a consequence of the method I was using to set the command line. If I call the test app from a batch file that specifies a command line, the limit is apparently 8190 bytes, which agrees reasonably with the minimum size of 8192 that VitualQuery returned in the MEMORY_BASIC_INFORMATION RegionSize member when passed the pointer returned by GetCommandLine. But curiously when I passed this command line, the RegionSize member was returned as 28672. And exceeding 8190 would cause the app to display and close, without apparently triggering an exception.

[jj2007]

in the explorer,he could pass any number of files you want.

Yep, that's right, I forgot.

[hutch--]

GetCL was written way back when GetCommandLine() could only access 128 bytes, it remains for compatibility reasons only as many people have used it over a long time.

Note the comment in the help file.

Comments

The buffer for the returned argument should be set at 128 bytes in length which is the maximum allowable.

use getcl_ex for much larger command lines.

getcl_ex

getcl_ex proc num:DWORD,pbuf:DWORD

[TouEnMasm]

Comments

The buffer for the returned argument should be set at 128 bytes in length which is the maximum allowable.

Ok ,this comment can be useful in the source code , if you read it.
I prefer to add this code

Code Select Expand


mov esi, lpCmdLine
    ;-------------- changed ------------------
    invoke lstrlen,esi
    .if eax > 600h -1
mov eax,5 ;buffer too small
      pop edi
      pop esi
      ret           
    .endif
    ;-----------------------------------------

You haven't to read it.

[jj2007]

i think the command line can be 32768, as i recall

That's almost correct, Dave - when passed with CreateProcess, 32766 bytes can be sent, exe file name included (Win7-32). I attach an interactive testbed.

[dedndave]

yah - i think more with unicorn mode   :P

[jj2007]

yah - i think more with unicorn mode   :P

Not really:

lpCommandLine [in, out, optional]
    The command line to be executed. The maximum length of this string is 32,768 characters, including the Unicorn terminating null character. If lpApplicationName is NULL, the module name portion of lpCommandLine is limited to MAX_PATH characters.