Why does this assembly program output the same string twice?

Question

This Caesar cipher program makes de-cipher string (length is always 10) and output is output.txt file. for example below code make .txt file,

ILIKEASSEM

HOWAREYOUU

but when i run this code, the file is

ILIKEASSEM

ILIKEASSEM

I can't know what is wrong even though debug by visual studio 2017 (MASM) debugger (T.T)

what is wrong...?

INCLUDE Irvine32.inc

.data
Num_Str DWORD 2
;;key = 10
Cipher_Str BYTE "SVSUOKCCOW",0 
        BYTE "RYGKBOIYEE",0
filename    BYTE"output.txt", 0
fileHandle  DWORD ?
BUFFER_SIZE = 12
buffer  BYTE BUFFER_SIZE DUP(?)
L   BYTE "ABCDEFGHIJKLMNOPQRSTUVWXYZ", 0
Decipher    BYTE "QRSTUVWXYZABCDEFGHIJKLMNOP", 0
count       DWORD ?

.code
main PROC

    mov     edx, OFFSET filename
    call    CreateOutputFile        
    mov     fileHandle, eax         



    mov     ecx, Num_Str
    mov     esi, 0                  

    L1:
        push    ecx                 
        mov     ecx, 11                 

    L2: 
        mov     al, Cipher_Str[esi]     
        sub     al, 65                  


        movzx   ax, al                  
        movzx   eax, ax                 

        mov     edi, eax                



        mov     al, Decipher[edi]       

        mov     buffer[esi], al     
        inc     esi

    loop    L2


        mov     bl, 13
        mov     buffer[esi], bl

        inc     esi
        mov     bl, 10
        mov     buffer[esi], bl


        mov     eax,fileHandle
        mov     edx,OFFSET buffer
        mov     ecx, BUFFER_SIZE
        call    WriteToFile



        pop     ecx



    loop    L1




    mov     eax, fileHandle 
    call    CloseFile


exit
main ENDP
END main

What did you observe in your debugging? You ought to be able to see what bytes are stored in `buffer`, where they came from, and how they were computed. Which part of this were you not able to follow? — Nate Eldredge, May 16 '20 at 19:47
Since you use `esi` for both reading and writing, the decoded version of the second string will be placed after the first one in the buffer (which is overflowed). — Jester, May 16 '20 at 19:48
If you simplify your code it would be fewer instructions to follow. e.g. the code in L2: could do `movzx eax, byte ptr Cipher_Str[esi]` / `mov al, Decipher[eax-65]` (then store AL into `buffer` and increment esi). There's no need to zero extend in 2 steps, from byte to word and then to dword, and you can do it for free as part of a load. (In fact it's *more* efficient to do a movzx load than to merge a new AL into the low byte of the old EAX. — Peter Cordes, May 16 '20 at 19:53

score 1 · Answer 1 · answered May 16 '20 at 21:33

You are overflowing the 12 byte buffer even before starting the 2nd iteration of the outer loop L1. You decipher the 10 characters but also the terminating zero (which you should pass over) and then add carriage return and linefeed. That's a total of 13 bytes!
You further overflow it because you don't reset the ESI register.

A possible solution is to reset to zero the offset register that indexes the output buffer with every iteration of the outer loop, and use a different index register to refer to the cipher strings that are 11 bytes apart.

    BUFFER_SIZE = 12
    ...
    mov     esi, offset Cipher_Str            
L1:
    xor     edi, edi            ; EDI=0
L2: 
    movzx   eax, byte ptr [esi+edi]
    mov     al, Decipher[eax-65]
    mov     buffer[edi], al 
    inc     edi
    cmp     edi, 10
    jb      L2
    mov     word ptr buffer[edi], 0A0Dh

    mov     eax, fileHandle
    mov     edx, OFFSET buffer
    mov     ecx, BUFFER_SIZE
    call    WriteToFile

    add     esi, 11             ; Move to the 2nd input string (or after it)
    cmp     esi, offset Cipher_Str + 22
    jb      L1

Why does this assembly program output the same string twice?

1 Answers1