The MASM Forum

General => The Campus => Topic started by: Grincheux on December 16, 2015, 04:26:10 AM

Title: Secure reading function
Post by: Grincheux on December 16, 2015, 04:26:10 AM
These two functions seem to be secured. I would like to have your advice. What could I improve?


                            .686p                     ; create 32 bit code
                        .MMX                      ; enable MMX instructions
                        .XMM                      ; enable SSE instructions
                        .NOCREF
                        OPTION casemap :none      ; case sensitive

;    -------------------------------------------------------------------------------------------------------------
                        .Code
;    -------------------------------------------------------------------------------------------------------------

;    ==================================================================================
                        ALIGN    16
;    ==================================================================================

ReadWholeFile            PROC    __lpszFileName:LPSTR
                        LOCAL    _lpMemory:LPBYTE
                        LOCAL    _FileHandle:HANDLE
                        LOCAL    _dwFileSize:DWORD
                        LOCAL    _dwTmp:DWORD

                        INVOKE    PathFileExists,__lpszFileName

                        test    eax,eax
                        jz        @Error_01

                            INVOKE    CreateFile,__lpszFileName,GENERIC_READ,0,NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL,NULL

                            cmp        eax,INVALID_HANDLE_VALUE
                            je        @Error_01

                                mov        _FileHandle,eax

                                INVOKE    GetFileSize,_FileHandle,ADDR _dwTmp

                                cmp        eax,INVALID_FILE_SIZE
                                je        @Error_02

                                    cmp        _dwTmp,0
                                    jne        @Error_02            ; File is too big

                                        or        eax,eax
                                        jz        @Error_02        ; File is empty

                                            mov        _dwFileSize,eax

                                            INVOKE    VirtualAlloc,NULL,eax,MEM_COMMIT + MEM_RESERVE,PAGE_EXECUTE_READWRITE

                                            test    eax,eax
                                            je        @Error_02

                                                mov        _lpMemory,eax
                                                lea        edx,_dwTmp

                                                INVOKE    ReadFile,_FileHandle,eax,_dwFileSize,edx,NULL

                                                test    eax,eax
                                                jz        @Error_03

                                                    mov        eax,_dwFileSize
                                                    sub        eax,_dwTmp
                                                    jnz        @Error_03

                                                        INVOKE    CloseHandle,_FileHandle

                                                        mov        eax,_lpMemory
                                                        ret

;    **********************************************************************************
                        ALIGN    16
;    **********************************************************************************

@Error_03 :        ; if eax = 0 we have reached the end of the file

                        push    MEM_RELEASE
                        push    0
                        push    _lpMemory
                        push    OFFSET @Error_02
                        jmp        VirtualFree

;    **********************************************************************************
                        ALIGN    16
;    **********************************************************************************

@Error_02 :

                        push    _FileHandle
                        push    OFFSET @Error_01
                        jmp        CloseHandle

;    **********************************************************************************
                        ALIGN    16
;    **********************************************************************************

@Error_01 :

                        xor        eax,eax
                        ret
ReadWholeFile            ENDP

;    ==================================================================================
                        ALIGN    16
;    ==================================================================================

File_Read                PROC    __FileHandle:HANDLE,__lpBuffer:LPBYTE,__dwNumberOfBytesToRead:DWord,__lpdwFlag:PTR DWord
                        LOCAL    _dwTmp:DWORD

                        lea        edx,_dwTmp

                        INVOKE    ReadFile,__FileHandle,__lpBuffer,__dwNumberOfBytesToRead,edx,NULL

                        test    eax,eax
                        jz        @Error_01

                            mov        eax,__dwNumberOfBytesToRead
                            sub        eax,_dwTmp
                            jnz        @Error_01

                                mov        eax,__lpdwFlag
                                mov        DWord Ptr [eax],TRUE            ; NO ERROR

                                mov        eax,__lpBuffer

                                ret

;    **********************************************************************************
                        ALIGN    16
;    **********************************************************************************

@Error_01 :

                        cmp        _dwTmp,0
                        jne        @Error

                            mov        eax,__lpdwFlag
                            mov        DWord Ptr [eax],FALSE                ; END OF FILE

                            mov        eax,__lpBuffer        ; End of file
                            ret                            ; That is not an error

@Error :

                        mov        eax,__lpdwFlag
                        mov        DWord Ptr [eax],-1                        ; ERROR WHILE READING

                        xor        eax,eax
                        ret
File_Read                ENDP


:dazzled:
Title: Re: Secure reading function
Post by: TouEnMasm on December 16, 2015, 04:57:36 AM

You use the exact size to allow memory,not very good (risk to read out of the memory ) .
Better is to round at least to the 16 bytes upper.
This avoid hole of memory (useless memory of a few bytes and win time)
You can use the fasted instructions using dword,qword,16 bytes registers.

Title: Re: Secure reading function
Post by: Grincheux on December 16, 2015, 05:00:42 AM
I use VirtualAlloc, so the size is rounded to a multiple of 4096.
The case you describe could appened,  that's true.
Avoiding this case would obliged to add 4096. I could spend a lot of memory.

I found a bug in eof detection.
The EOF is detected when ReadFile returns TRUE and the number of bytes read is 0.

New version (http://www.phrio.biz/download/$File_Read.zip)
Title: Re: Secure reading function
Post by: TouEnMasm on December 16, 2015, 05:08:14 AM

To avoid the need of 4096 bytes,globalalloc or heapalloc are perfect.
Title: Re: Secure reading function
Post by: Grincheux on December 16, 2015, 06:00:03 AM
QuoteNote :  The global functions have greater overhead and provide fewer features than other memory management functions. New applications should use the heap functions (https://msdn.microsoft.com/en-us/library/windows/desktop/aa366711(v=vs.85).aspx) unless documentation states that a global function should be used.

But I prefer Virtual Functions.

I have added the SetErrorMode function with the parameter SEM_FAILCRITICALERRORS.
The attachment below is updated. Downloading this file always gives the last version.
Title: Re: Secure reading function
Post by: jj2007 on December 16, 2015, 07:50:23 AM
You can use VirtualAlloc, no problem. Just use filesize+1 to care for the zero delimiter. It will not waste memory.

Note, though, that
a) for small allocations VA is very slow compared to HeapAlloc
b) for big allocations (>0.5MB) HeapAlloc uses VirtualAlloc internally.
Therefore, to get always the best possible performance, you can use HeapAlloc. Microsoft says that, not me - I just explain why they are saying that ;-)
Title: Re: Secure reading function
Post by: Grincheux on December 17, 2015, 07:49:50 AM
I try to use HeapAlloc :



add eax,256
push eax
push HEAP_ZERO_MEMORY + HEAP_NO_SERIALIZE
call GetProcessHeap
push eax
call HeapAlloc



Works well, but HeapFree returns 0!




push _lpMemory
push HEAP_NO_SERIALIZE
call GetProcessHeap
push eax
push OFFSET @ReadWholeFile_Error_02
jmp HeapFree


Title: Re: Secure reading function
Post by: dedndave on December 17, 2015, 08:32:30 AM
QuoteHEAP_NO_SERIALIZE
This value should not be specified when accessing the process's default heap.

https://msdn.microsoft.com/en-us/library/windows/desktop/aa366597%28v=vs.85%29.aspx (https://msdn.microsoft.com/en-us/library/windows/desktop/aa366597%28v=vs.85%29.aspx)

and, this is crazy code   :biggrin:
    push    OFFSET @ReadWholeFile_Error_02
    jmp     HeapFree


try this....
        INCLUDE    \Masm32\Include\Masm32rt.inc

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

        .DATA?

hHeap   HANDLE ?
lpBlock LPVOID ?

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

        .CODE

;***********************************************************************************************

main    PROC

        INVOKE  GetProcessHeap
        mov     hHeap,eax

        INVOKE  HeapAlloc,eax,HEAP_ZERO_MEMORY,120000
        mov     lpBlock,eax
        print   uhex$(eax),13,10,13,10

        inkey

        INVOKE  HeapFree,hHeap,NULL,lpBlock
        print   uhex$(eax),13,10,13,10

        inkey
        exit

main    ENDP

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

        END     main
Title: Re: Secure reading function
Post by: Grincheux on December 17, 2015, 04:13:09 PM
Quotemov     lpBlock,eax[/size]

[/size]
[/size][size=78%]Rather than making this I made : MOV EAX,lpBlock[/size]
[/size][size=78%]So when I wanted to free the block the address was FALSE.[/size]
Title: Re: Secure reading function
Post by: Grincheux on December 17, 2015, 05:13:36 PM
I am pleased to have finished this function!
The last and documented version is available here (http://www.phrio.biz/download/$File_Read.7z)
Thanks everyone for your help. (Dedndae, Jj2007, ToutEnAsm)
Title: Re: Secure reading function
Post by: dedndave on December 17, 2015, 05:38:05 PM
aligning the return addresses isn't really necessary...

push OFFSET @Error_02
jmp HeapFree

; **********************************************************************************
ALIGN 4
; **********************************************************************************

@Error_02 :

push _FileHandle
push OFFSET @Error_01
jmp CloseHandle

; **********************************************************************************
ALIGN 4
; **********************************************************************************

@Error_01 :


while it may speed up the RET (slightly), it is probably slower than CALL
Title: Re: Secure reading function
Post by: Grincheux on December 17, 2015, 06:00:43 PM
In the last test JJ2007 did this code is quicker on AMD CPU


Quote
   push      _FileHandle
   push      OFFSET @Error_01
   jmp      CloseHandle


Agner Frog tells that if a call is followed by a jump, it can be replaced by a jump.
The is the answer to the "Crazy Code".
Title: Re: Secure reading function
Post by: dedndave on December 18, 2015, 01:28:01 AM
yah - i don't really think that's valid
it's difficult to time an API function - let alone the return method
and - that's a lot of work to code each call that way
better to dereference the calls - lol

but, whatever floats your boat
Title: Re: Secure reading function
Post by: TouEnMasm on December 18, 2015, 01:40:16 AM
interesting and improve is this one:
http://masm32.com/board/index.php?topic=3483.msg36676#msg36676
accept any types and any size of datas
wrong is the assertion:
Quote
If it is data that can be fitted into fixed length areas of memory, the task is simple, you allocate an array of pointers, then a block of memory and aim each pointer to each section of memory.
I  use it more often with lines of text (words) of various sizes (just add a zero at end before save them)
to empty the buffer is also easy:
invoke WriteToBlocHeap,addr BLOCHEAP,0,0  ;empty the block
a format can be also be used to save datas:
FORMAT STRUCT
       datalen DWORD
       thedata db datalen dup (?) ;pseudo code
FORMAT ENDS

this give:
mov edx,FORMAT.datalen
add edx,sizeof FORMAT.datalen (4)
invoke WriteToBlocHeap,addr BLOCHEAP,addr FORMAT,edx


Title: Re: Secure reading function
Post by: dedndave on December 18, 2015, 03:41:58 AM
...and....

there are roughly 5 times as many intel cpu's running windows as there are amd   :P
Title: Re: Secure reading function
Post by: Grincheux on December 18, 2015, 04:54:27 AM
There was something important that was missing. You read a file, ok, but how long is the buffer?
Now you pass the address of a DWORD into which the file size (which also is the buffer size - 256) is returned.
The pointer is initialized to zero at the function entry.


New version (7zip file) (http://phrio.biz/download/$File_Read.7z)


NO VIRUS FOUND INTO THIS FILE

Analyze link (https://www.virustotal.com/fr/file/226248ca9c8a113118f8640d4f559c595f247220fc80f5bbc9080cc39fb17846/analysis/1450374686/)
Title: Re: Secure reading function
Post by: TouEnMasm on December 18, 2015, 07:06:56 AM
ReadFile PROTO hFile:HANDLE ,lpBuffer:XMASM ,nNumberOfBytesToRead:DWORD ,lpNumberOfBytesRead:XMASM ,lpOverlapped:XMASM

use the lpNumberOfBytesRead to get the answer and made a verify all is well done
substract this number to the size of the buffer and you have the rest of memory
Title: Re: Secure reading function
Post by: Grincheux on December 18, 2015, 07:12:43 AM
INVOKE ReadFile,_FileHandle,eax,_dwFileSize,edx,NULL

test eax,eax
jz @Error_03

mov eax,_dwFileSize

sub eax,_dwTmp ; If the number of bytes read is not equal
jnz @Error_03 ; to the number of bytes to read => ERROR

mov eax,__lpdwFileSize
mov edx,_dwFileSize
mov DWord Ptr [eax],edx

INVOKE CloseHandle,_FileHandle

mov eax,_lpMemory
ret


That's what I do
Title: Re: Secure reading function
Post by: TWell on December 18, 2015, 07:36:29 AM
Do you mean allocated buffer?
HeapSize()?
Title: Re: Secure reading function
Post by: Grincheux on December 18, 2015, 07:41:37 AM
It's the size of the file.
The size of the allocated buffer is equal to the size of the file plus 256 bytes.
Title: Re: Secure reading function
Post by: TouEnMasm on December 18, 2015, 06:52:35 PM
It's unclear.
Wich size do you search ??????????
Title: Re: Secure reading function
Post by: Grincheux on December 18, 2015, 08:58:18 PM
The file size
Title: Re: Secure reading function
Post by: hutch-- on December 19, 2015, 12:56:23 AM
Philippe,

The normal sequence is you open a file, get its length, allocate a buffer that is large enough then read the file into the buffer.

Just look up the foillowing in the "macros.asm" file in MASM32 and you will find all of the useful API functions for doing file IO. Look for this in the macros file,

  ; **************
  ; File IO Macros
  ; **************
Title: Re: Secure reading function
Post by: TouEnMasm on December 19, 2015, 01:20:31 AM
I use this one since a certain time:
comments are in french


;   le batch pour batir une librairie
;batch,sources et .inc (contenant les proto)
FichMem    struc
               Hmem     dd ?
               Mpoint   dd ?
               Taille   dd ?
        FichMem    ends

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

      .386                      ; force 32 bit code
      .model flat, stdcall      ; memory model & calling convention
      option casemap :none      ; case sensitive
      ; --------------------------------
      ; Les fichiers inclus içi
      ; --------------------------------
       include \masm32\include\windows.inc
       include \masm32\include\kernel32.inc
       include \masm32\include\user32.inc
        include macros.txt

      ; ---------------------------------------------------------
      ; Les PROTO sont tous regroupés dans un fichier .inc du même
      ; nom que la librairie .lib
      ; ---------------------------------------------------------

.data
ercharge db "Taille ",0
texthou db " Handle fichier ",0
textHmem db "Handle memoire ",0
textPm db "Pointeur memoire ",0
textlu db " Nb octets en lecture ",0
InfosFiles WIN32_FIND_DATA <>

    .code
;################################################################
;----------- pushad popad -----------------
LoadFile proc uses ebx NomAediter:DWORD,PileMem:DWORD
LOCAL retour:DWORD
LOCAL fichier:DWORD
local hMemory:DWORD
local pMemory:DWORD
local hFile:DWORD
local NbOctetsaLire:DWORD
local NbOctetsLus:DWORD

          ;pilemem adresse d'une structure FichMem
   ;rajoute un zero en fin de fichier
   ;nécessaire dans les copies avec clipboard par exemple
   ;utilise  WIN32_FIND_DATA et NomComplet

;FichMem    struc
; Hmem     dd ?
; Mpoint   dd ?
; Taille   dd ?
;FichMem    ends

mov ebx,PileMem
          invoke RtlZeroMemory,addr InfosFiles,sizeof InfosFiles
          invoke FindFirstFile,NomAediter,addr InfosFiles

          ;----------------------------------------------------------
          ; taille du fichier nFileSizeHigh; DWORD    nFileSizeLow WIN32_FIND_DATA
          ;   nom court ou nom long sans chemin ?         
          ;-----------------------------------------------------------
         
          .if eax==INVALID_HANDLE_VALUE
               mov retour,0         
       jmp FindeLoadFile
   .else
    mov edx,eax
          invoke FindClose,edx
               mov retour,1    
          .endif
         ;------------------------------------------------------         
         ; on connait maintenant toutes les infos sur le fichier         
         ;-------------------------------------------------------


;----------------------------
;  ouvrir le fichier
;----------------------------
        invoke CreateFile,NomAediter,GENERIC_READ,NULL,\
                        NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL,0
.if eax==INVALID_HANDLE_VALUE
;invoke  MessageBox,NULL,SADD(" Erreur d'ouverture Fichier "),\
; SADD("LoadFile"),MB_OK
mov retour,0
jmp FindeLoadFile
.endif
mov hFile,eax

;-------------------------
;obtenir le handle de mémoire, taille du fichier dw
;-------------------------
mov ecx,InfosFiles.nFileSizeLow
mov dword ptr (FichMem ptr [ebx]).Taille,ecx
inc ecx ;pour rajouter le zero en fin de fichier
add ecx,32 ;mmx utilise des registres de 16 bytes
and ecx,0FFFFFFF0h ;rounded 16 bytes upper
invoke GlobalAlloc,GMEM_MOVEABLE or GMEM_ZEROINIT,ecx

.if eax == 0
;invoke  MessageBox,NULL,SADD("Pas assez de place mémoire "),\
; SADD("GlobalAlloc"),MB_OK
mov retour,0
jmp FindeLoadFile
.endif           
mov  hMemory,eax
mov dword ptr (FichMem ptr [ebx]).Hmem,eax
;obtenir le pointeur de memoire
invoke GlobalLock,hMemory
mov  pMemory,eax
mov dword ptr (FichMem ptr [ebx]).Mpoint,eax

mov eax,InfosFiles.nFileSizeLow 
mov NbOctetsaLire,eax
invoke ReadFile,hFile,pMemory,NbOctetsaLire,addr NbOctetsLus,NULL

;-------------------------
;libéré la memoire ,fermé le fichier ouvert
;-------------------------
; invoke GlobalUnlock,pMemory
; invoke GlobalFree,hMemory
invoke CloseHandle,hFile
mov retour,1

FindeLoadFile:
mov eax,retour
ret
           

LoadFile endp

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


end

Title: Re: Secure reading function
Post by: Grincheux on December 19, 2015, 04:18:21 AM
I think there is a mistake. Into one of my post I told that I had forgotten the file size. If the buffer needed to be written I did know its size. But I had the information because I call "GetFileSize" function.


I put the entire source code. You can download it here (http://www.phrio.biz/download/$File_Read.zip)
I appreciate any comment and speed tests. I don't know how to measure how fast/slow the functions are.

.686p                     ; create 32 bit code
.MMX                      ; enable MMX instructions
.XMM                      ; enable SSE instructions
.NOCREF
OPTION casemap :none      ; case sensitive

; -------------------------------------------------------------------------------------------------------------
.Code
; -------------------------------------------------------------------------------------------------------------

; ==================================================================================
ALIGN 4
; ==================================================================================

; ReadWholeFile : INPUT PARAMETER : A pointer on the file name to be read.
; RETURNED VALUE : A pointer on the allocated memory block
; Or NULL if there was an error.

; The memory block must be free by the caller usif HeapFree
; The input file is opened for reading.
; During the read operation all access to the file will fail because there is no share.
; The file must not be empty and its size must be in the range 0...FFFFFFFFh

; Memory allocated size : File size + 256 bytes
; When the fuction returns the input file is closed even if there was an error
; Modified registers : ALL except EBX, EDI and ESI which are not used.

ReadWholeFile PROC __lpszFileName:LPSTR
LOCAL _lpMemory:LPBYTE ; The adress of the returned memory block
LOCAL _FileHandle:HANDLE ; The file handle
LOCAL _dwFileSize:DWORD ; Low part of the file size
LOCAL _dwTmp:DWORD ; Temporary variable

INVOKE PathFileExists,__lpszFileName

test eax,eax ; File does not exist
jz @Error_01

INVOKE CreateFile,__lpszFileName,GENERIC_READ,0,NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL,NULL

cmp eax,INVALID_HANDLE_VALUE ; Can't open the file
je @Error_01

lea edx,_dwTmp
mov _FileHandle,eax

INVOKE GetFileSize,eax,edx

cmp eax,INVALID_FILE_SIZE
je @Error_02

cmp _dwTmp,0
jne @Error_02 ; File is too big

or eax,eax
jz @Error_02 ; File is empty

mov _dwFileSize,eax

add eax,256 ; File size + 256
push eax
push HEAP_ZERO_MEMORY ; Memory initialyzed with '0'
call GetProcessHeap
push eax
call HeapAlloc

test eax,eax ; Memory allocation failed?
je @Error_02

mov _lpMemory,eax
lea edx,_dwTmp

INVOKE ReadFile,_FileHandle,eax,_dwFileSize,edx,NULL

test eax,eax
jz @Error_03

mov eax,_dwFileSize

sub eax,_dwTmp ; If the number of bytes read is not equal
jnz @Error_03 ; to the number of bytes to read => ERROR

@EndOfFile :

INVOKE CloseHandle,_FileHandle

mov eax,_lpMemory
ret

; **********************************************************************************
ALIGN 4
; **********************************************************************************

@Error_03 :

call GetProcessHeap

push _lpMemory
push 0
push eax
push OFFSET @Error_02
jmp HeapFree

; **********************************************************************************
ALIGN 4
; **********************************************************************************

@Error_02 :

push _FileHandle
push OFFSET @Error_01
jmp CloseHandle

; **********************************************************************************
ALIGN 4
; **********************************************************************************

@Error_01 :

xor eax,eax
ret
ReadWholeFile ENDP

; ==================================================================================
ALIGN 4
; ==================================================================================

File_Read PROC __FileHandle:HANDLE,__lpBuffer:LPBYTE,__dwNumberOfBytesToRead:DWord,__lpdwFlag:PTR DWord
LOCAL _dwTmp:DWORD

lea edx,_dwTmp

INVOKE ReadFile,__FileHandle,__lpBuffer,__dwNumberOfBytesToRead,edx,NULL

test eax,eax
jz @Error_01

cmp _dwTmp,0
je @EndOfFile

mov eax,__dwNumberOfBytesToRead

sub eax,_dwTmp
jnz @Error_01

@EndOfFile : ; EOF detected

mov eax,__lpdwFlag
mov edx,_dwTmp
mov DWord Ptr [eax],edx ; NO ERROR - Returns the number of bytes read

mov eax,__lpBuffer

ret

; **********************************************************************************
ALIGN 4
; **********************************************************************************

@Error_01 :

mov eax,__lpdwFlag
mov DWord Ptr [eax],-1 ; ERROR WHILE READING

xor eax,eax
ret
File_Read ENDP