News:

MASM32 SDK Description, downloads and other helpful links
MASM32.com New Forum Link
masmforum WebSite

File Shredder

Started by Bieb, January 19, 2005, 10:57:41 PM

Previous topic - Next topic

Bieb

Okay, I'm working on a simple file shredder program.  Currently, all it does is take a filename in an EditBox, fill the file with useless data, close the file, and delete the file.  However, my code is just crashing, not doing anything useful.

The code, taken out of on Easy Code project with an Edit Box (IDC_WINDOW1_FILEBOX) and a button (IDC_WINDOW1_SHREDBUTTON) is as follows.


.Const

.Data?
FilePath Byte 200 Dup (?)
hFile DWord ?
FileBoxHandle HWND ?
hWord Word ?
lWord Word ?
.Data
DataByte DB 01010101B
.Code

Window1Procedure Proc Private hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
.If uMsg == WM_CREATE
Invoke GetWindowItem, hWnd, IDC_WINDOW1_FILEBOX
Mov Eax, FileBoxHandle

.ElseIf uMsg == WM_COMMAND
Mov Eax, wParam
.If Ax == IDC_WINDOW1_SHREDBUTTON
Shr Eax, 16
.If Ax == BN_CLICKED
Invoke GetText, FileBoxHandle, Addr FilePath
Invoke CreateFile, Addr FilePath, GENERIC_READ Or GENERIC_WRITE, NULL, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL
Mov hFile, Eax
Invoke GetFileSize, hFile, hWord
Mov Ebx, Eax
LoopLabel:
Invoke WriteFile, hFile, DataByte, 1, NULL, NULL
Dec Ebx
Jnz LoopLabel
Invoke CloseHandle, hFile
Invoke DeleteFile, Addr FilePath

.EndIf
.EndIf

.ElseIf uMsg == WM_CLOSE
Invoke IsModal, hWnd
.If Eax
Invoke EndModal, hWnd, IDCANCEL
Return TRUE
.EndIf
.Endif
Return FALSE
Window1Procedure EndP



Window1FileBox Proc Private hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
Return FALSE
Window1FileBox EndP


If it's of any help, I've also attached the entire project.

[attachment deleted by admin]

petezl

Hi Bleb,
Sorry, I don't understand that easycode stuff but you seem to have a missing value "PassWord="
Peter.
Cats and women do as they please
Dogs and men should realise it.

pbrennick

Bieb,
At a quick glance, shouldn't

Mov  Eax, FileBoxHandle

be

Mov   FileBoxHandle, Eax

Paul

Relvinian

The things I noticed which may or may not cause problems are:

Not checking return values on API calls (specifically, CreateFile).  If the filename specifiy doesn't exist, you are still trying to write data to it.
Once you open the file for writing, usuallly, it is best to always set the file position before writing unless you ALWAYS want to write at the beginning.

You are using the EBX register without preserving it.

At the top of your code, you are trying to move the FileBoxHandle into EAX.  Should be the other way around

Your buffer for the filename is only 200 bytes -- is that enoug.  Should be big enough to hold MAX_PATH (which is 260 btw).


Hope this helps.

Relvinian


Bieb

Okay, for anyone who doesn't know, the purpose of this code is to delete a file in such a way that it can't be recovered.  This doesn't do a very good job of it, but I can take care of that once I have the groundwork done.  I fixed the inverted Mov operands, and used Ecx instead of Ebx, and I ended up with this code.


.Const

.Data?
FilePath Byte 200 Dup (?)
hFile DWord ?
FileBoxHandle HWND ?
hWord Word ?
lWord Word ?
.Data
DataByte DB 01010101B
mbText DB "The file has been shredded", 0
mbCaption DB "Done", 0
.Code

Window1Procedure Proc Private hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
.If uMsg == WM_CREATE
Invoke GetWindowItem, hWnd, IDC_WINDOW1_FILEBOX
Mov FileBoxHandle, Eax

.ElseIf uMsg == WM_COMMAND
Mov Eax, wParam
.If Ax == IDC_WINDOW1_SHREDBUTTON
Shr Eax, 16
.If Ax == BN_CLICKED
Invoke GetText, FileBoxHandle, Addr FilePath
Invoke MessageBox, NULL, Addr FilePath, Addr FilePath, MB_OK
Invoke CreateFile, Addr FilePath, GENERIC_READ Or GENERIC_WRITE, NULL, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL
Mov hFile, Eax
Invoke GetFileSize, hFile, hWord
Mov Ecx, Eax
Dec Ecx
LoopLabel:
Invoke WriteFile, hFile, DataByte, 1, NULL, NULL
Dec Ecx
Jnz LoopLabel
Invoke CloseHandle, hFile
Invoke DeleteFile, Addr FilePath
Invoke MessageBox, NULL, Addr mbText, Addr mbCaption, MB_OK

.EndIf
.EndIf

.ElseIf uMsg == WM_CLOSE
Invoke IsModal, hWnd
.If Eax
Invoke EndModal, hWnd, IDCANCEL
Return TRUE
.EndIf
.Endif
Return FALSE
Window1Procedure EndP



Window1FileBox Proc Private hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
Return FALSE
Window1FileBox EndP


In this, I added two message boxes to confirm the program's operation.  The first to verify that it got the file name alright, and the second to notify me it has finished it's work.  I gave it a valid file name, and I get that back in the first confirmation message box, but the program crashes somewhere between the first and second message boxes, with no changes having been made to the file.  Any ideas?

tenkey

#5
Now that you're using ECX, you've got the other register preservation problem. WriteFile doesn't guarantee that ECX will remain unchanged.

WriteFile requires the address of a data buffer. But you are passing the actual data (+ extra bytes), and it's being treated as an address. Add the ADDR operator. I believe you also need to provide a valid address for the lpNumberOfBytesWritten argument.

Invoke WriteFile, hFile, Addr DataByte, 1, Addr dwBytesWritten, NULL

.data?
dwBytesWritten DWord ?
A programming language is low level when its programs require attention to the irrelevant.
Alan Perlis, Epigram #8

hutch--

Bieb,

You buy yourself a lot of grief trying to use registers as variables mixed with HLL API calls. You would bother if there was some gain but in this context there is not, its a technique best used in algo design where you are mainly working in registers and instructions only.

What I would suggest is simply make some LOCAL variables that hold the return values and then you don't have the register overwrite problem. Also note that if you are only using a variable within one procedure, there is no point putting it in the .DATA(?) section and you keep the procedure self contained by using a LOCAL where you can.
Download site for MASM32      New MASM Forum
https://masm32.com          https://masm32.com/board/index.php

Bieb

Alright, I'll go and do all that LOCAL stuff once I've gotten the basic concept working.  Here's a version that works with only two flaws


.Const

.Data?
FilePath Byte 200 Dup (?)
hFile DWord ?
FileBoxHandle HWND ?
hWord Word ?
lWord Word ?
ByteWrite DWord ?
.Data
DataByte DB 01010101B
mbText DB "The file has been shredded", 0
mbCaption DB "Done", 0
.Code

Window1Procedure Proc Private hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
.If uMsg == WM_CREATE
Invoke GetWindowItem, hWnd, IDC_WINDOW1_FILEBOX
Mov FileBoxHandle, Eax

.ElseIf uMsg == WM_COMMAND
Mov Eax, wParam
.If Ax == IDC_WINDOW1_SHREDBUTTON
Shr Eax, 16
.If Ax == BN_CLICKED
Invoke GetText, FileBoxHandle, Addr FilePath
Invoke MessageBox, NULL, Addr FilePath, Addr FilePath, MB_OK
Invoke CreateFile, Addr FilePath, GENERIC_READ Or GENERIC_WRITE, NULL, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL
.If Eax == INVALID_HANDLE_VALUE
Invoke MessageBox, NULL, Addr FilePath, Addr FilePath, MB_OK
.EndIf
Mov hFile, Eax
Invoke GetFileSize, hFile, hWord
Mov Ecx, 53 ; Hard coded file length
Dec Ecx
LoopLabel:
Push Ecx
Invoke WriteFile, hFile, Addr DataByte, 1, Addr ByteWrite, NULL
Pop Ecx
Dec Ecx
Jnz LoopLabel
Invoke CloseHandle, hFile
Invoke DeleteFile, Addr FilePath
Invoke MessageBox, NULL, Addr mbText, Addr mbCaption, MB_OK

.EndIf
.EndIf

.ElseIf uMsg == WM_CLOSE
Invoke IsModal, hWnd
.If Eax
Invoke EndModal, hWnd, IDCANCEL
Return TRUE
.EndIf
.Endif
Return FALSE
Window1Procedure EndP



Window1FileBox Proc Private hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
Return FALSE
Window1FileBox EndP


The most important one is that my GetFileLength call isn't working right.  Unless I hard code the length of the file into the program, it goes into an endless loop, and the file just keeps getting bigger.  The other is, once it's finished it all, the call to DeleteFile isn't doing anything.

pbrennick

#8
Bieb,
In reference to the GetFileSize call, iif it fails Eax will contain 0xFFFFFFFFh, that is why the file keeps growing.  If this is happening then the problem has to be with hFile.
Paul

pbrennick

I also think that

FilePath Byte 200 Dup (?)

should be

FilePath Byte 200 Dup (0)  (and move it from the .Data? section to the .Data  section)

That makes sure that there is a null terminator at the end of the filename, this is probably why DeleteFile is not working.

Paul

Ramon Sala

Hi Bieb,

I found the problem in your code. Here's the code that works (file is attached):


.Const

.Data?
FilePath Byte 200 Dup (?)
hFile DWord ?
FileBoxHandle HWND ?
hWord DWord ?
lWord DWord ?
ByteWrite DWord ?
.Data
DataByte DB 01010101B
mbText DB "The file has been shredded", 0
mbCaption DB "Done", 0
.Code

Window1Procedure Proc Private hWnd:HWND, uMsg:ULONG, wParam:WPARAM, lParam:LPARAM
   .If uMsg == WM_CREATE
      Invoke GetWindowItem, hWnd, IDC_WINDOW1_FILEBOX
      Mov FileBoxHandle, Eax
   
   .ElseIf uMsg == WM_COMMAND
      Mov Eax, wParam
      .If Ax == IDC_WINDOW1_SHREDBUTTON
         Shr Eax, 16
         .If Ax == BN_CLICKED
            Invoke GetText, FileBoxHandle, Addr FilePath
            Invoke MessageBox, NULL, Addr FilePath, Addr FilePath, MB_OK
            ;Invoke CreateFile, Addr FilePath, GENERIC_READ Or GENERIC_WRITE, NULL, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL
            Invoke CreateFile, Addr FilePath, GENERIC_READ Or GENERIC_WRITE, NULL, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL
            .If Eax == INVALID_HANDLE_VALUE
               Invoke MessageBox, NULL, Addr FilePath, Addr FilePath, MB_OK
               Return TRUE
            .EndIf
            Mov hFile, Eax
            Invoke GetFileSize, hFile, Addr hWord ;hWord
            Mov Ecx, 53 ; Hard coded file length
            Dec Ecx
LoopLabel:
            Push Ecx
            Invoke WriteFile, hFile, Addr DataByte, 1, Addr ByteWrite, NULL
            Pop Ecx
            Dec Ecx
            Jnz LoopLabel
            Invoke CloseHandle, hFile
            Invoke DeleteFile, Addr FilePath
            Invoke MessageBox, NULL, Addr mbText, Addr mbCaption, MB_OK
           
         .EndIf
      .EndIf   
     
   .ElseIf uMsg == WM_CLOSE
      Invoke IsModal, hWnd
      .If Eax
         Invoke EndModal, hWnd, IDCANCEL
         Return TRUE
      .EndIf
   .Endif
   Return FALSE
Window1Procedure EndP



The  second parameter in GetFileSize function (line 35) must be the address of a DWord value (not the value but the address and not Word but DWord). So, the hWord i lWord variables has been changed to DWord values (lines 7 and 8) and the parameter passed to the function has been changed to Addr hWord. This parameter can also be NULL if you know the file is not large enough.

The CreateFile function (line 28) has the flag OPEN_EXISTING which means that the file you're going to open has to exist. If not, an INVALID_HANDLE_VALUE error is returned. If you want to open (create) a file that does not exist, this flag should be CREATE_ALWAYS (I commented that line an added another one which creates the file).

Finally, and just as a precaution, if an INVALID_HANDLE_VALUE error is returned from CreateFile function, we should return TRUE (line 32) in order to avoid the code below to be executed. In fact, we should return at that point (TRUE or FALSE are valid). Returning TRUE means no further processing.

Regards,

Ramon


[attachment deleted by admin]
Greetings from Catalonia

Ramon Sala

#11
Bieb,

After posting my replay for the "File Shredder" problem, I realized that you wanted to obtain the low and high words of the high-order DWord of the file size (GetFileSize function). As that DWord value can be divided into two words, your original code in .Data? section was right:

hWord    Word    ?
lWord    Word    ?


But you have to pass the address of a DWord variable using Addr operator (Addr hWord), not its value. Anyway, in most cases, you can pass a NULL value instead, as that DWord address has only sense for very large files (more then 2^32 bytes).

To avoid errors in API calls needing an address to be passed, watch the context help for Windows API appearing under the line your writing. When a parameter refers to an address, it is shown in the context help with the Addr operator.

About CreateFile function, you can use the OPEN_EXISTING flag only if the file you're going to open exists. I changed it to CREATE_ALWAYS just for testing.


Regards,

Ramon
Greetings from Catalonia

Ramon Sala

#12
Hi Bieb,

Paul E. Brennnick and I saw another thing which might cause problems. When calling the GetFileSize function, if it returns 0 or INVALID_HANDLE_VALUE (which is -1), your program would crash when entering the loop. It doesn't happen now beacuse you directly move the value 53 to Ecx:


    Invoke GetFileSize, hFile, Addr hWord ;hWord
    Mov Ecx, 53 ; Hard coded file length
    Dec Ecx


But if you get the value returned by GetFileSize function and it was 0 or -1, your program would crash. So the right code for this part is the following:

            Invoke GetFileSize, hFile, Addr hWord   ;hWord
            Cmp Eax,0
            Jle @F
            Mov Ecx, Eax ; 53 ; Hard coded file length
LoopLabel:
            Push Ecx
            Invoke WriteFile, hFile, Addr DataByte, 1, Addr ByteWrite, NULL
            Pop Ecx
            Dec Ecx
            Jnz LoopLabel
@@:         Invoke CloseHandle, hFile
            Invoke DeleteFile, Addr FilePath



If the file is not found the -1 value (INVALID_HANDLE_VALUE) will be returned. When the file is found but it's empty (0 bytes), the 0 value is returned, so you have to take this into account. That's why the returned value is checked before entering the loop (see the code above). If it is lower or equal than 0, then the loop is not executed.


Regards,

Ramon
Greetings from Catalonia

Ramon Sala

#13
Hi,

For everything said in this topic I would to make clear some considerations in order to use the Easy Code IDE properly and avoiding undesired behaviors:


- When calling any Easy Code method returning a string (i.e. GetText method), an ASCIIZ string is returned in the specified buffer, that is, it includes the final zero. So, you do not need to initialize the buffer contents to zero before the call. To prevent errors, always make this buffer 256 bytes long unless you know the exact text length (plus a byte for the zero character). There is no Easy Code method for retreiving the text length of an object, but you can send a WM_GETTEXTLENGTH message to find it out.

- Registers Eax, Ebx, Ecx, Edx, Edi and Esi can be used freely inside the window procedure of an Easy Code object, as they are preserved before entering the procedure and restored after leaving it. You just have to take care of registers along your code according to your needs.

- For any call to a Windows API function, remember that only Ebx, Edi and Esi registers are preserved (but not Ecx and Edx).

- All Easy Code methods preserve Ebx, Ecx, Edx, Edi and Esi registers. Only the Eax register changes as it is used for the return value.

- The defaut return value (FALSE) for an object procedure (Window or Control) has never to be removed or changed in order to avoid crashes. If you process a message and do not want any further processing for it, just return TRUE (only for that message).

- When the procedure for a Control object is not used because you can do all work in the WM_COMMAND and/or WM_NOTIFY messages received by its owner window (that's all you need in most cases), remove the entire Control procedure in order to save some bytes in the final executable file.

- .Code directive must always exist and be after .Const, .Data? and .Data in order the IDE to work properly (controlling variables, structures, macros and procedures). All code being before the .Code directive, is considered to be data.


Regards,

Ramon
Greetings from Catalonia

Bieb

Thanks.  If I end up with a very large file length in a qword, will the dec instruction work with it?