Author Topic: Using scanf crashes the program [Windows]  (Read 9686 times)

Offline PurityLake

  • Jr. Member
  • *
  • Posts: 4
Using scanf crashes the program [Windows]
« on: September 02, 2014, 12:13:32 AM »
Code: [Select]
global _main
extern _printf
extern _scanf

section .data
message: db "Hello World!", 0xA, 0x0
formatin: db "%s\n", 0xA, 0x0

section .bss
input: resb 32

section .text
_main:
push dword [input]
push formatin
call _scanf
ret


This is the code I am using. It works up to the point of where I give the input, after which the program stops responding. Compile and link the program with:

Code: [Select]
nasm -f win32 scan.asm
gcc scan.obj

Offline Frank Kotler

  • NASM Developer
  • Hero Member
  • *****
  • Posts: 2667
  • Country: us
Re: Using scanf crashes the program [Windows]
« Reply #1 on: September 02, 2014, 02:10:41 AM »
Hi PurityLake,

The "main" (cough) problem I see is that you haven't "cleaned up " or "balanced" the stack after _scanf returns. The Windows API is "stdcall" - callee cleans up - but the C library is "cdecl" - caller cleans up. If you don't understand this, look up "calling conventions".

A second problem is that you've pushed the "[contents]" of your buffer ("input"). You want the address (no "[]").

There may be a problem with the way you've declared the format string. Nasm will recognize "\n" (and others) IFF you use "back quotes" ('`') - with double quotes or single "forward" quotes, you'll just get a literal "\n", which _scanf may not like.

I should point out that you've got a potential buffer overflow. Since it's not on the stack, it isn't easily exploitable and may not be a problem.

Code: [Select]
global _main
extern _printf
extern _scanf

section .data
message: db "Hello World!", 0xA, 0x0
; formatin: db "%s\n", 0xA, 0x0
formatin: db "%s", 0x0

section .bss
input: resb 32

section .text
_main:
; push dword [input]
push input
push formatin
call _scanf
        add esp, 4 * 2  ; clean up stack!
; now you can safely "ret"
ret
That's untested, but I "think" it'll work. Let us know if any further problems...

Best,
Frank


Offline PurityLake

  • Jr. Member
  • *
  • Posts: 4
Re: Using scanf crashes the program [Windows]
« Reply #2 on: September 02, 2014, 09:27:42 PM »
Hi Frank,

Perfect, your code has sorted my problems.

I am very new to assembly in general so I am a little iffy on some of the details but I will get there. Thanks again for your help and your advice.

Offline PurityLake

  • Jr. Member
  • *
  • Posts: 4
Re: Using scanf crashes the program [Windows]
« Reply #3 on: September 02, 2014, 11:14:51 PM »
Hi PurityLake,

The "main" (cough) problem I see is that you haven't "cleaned up " or "balanced" the stack after _scanf returns. The Windows API is "stdcall" - callee cleans up - but the C library is "cdecl" - caller cleans up. If you don't understand this, look up "calling conventions".

A second problem is that you've pushed the "[contents]" of your buffer ("input"). You want the address (no "[]").

There may be a problem with the way you've declared the format string. Nasm will recognize "\n" (and others) IFF you use "back quotes" ('`') - with double quotes or single "forward" quotes, you'll just get a literal "\n", which _scanf may not like.

I should point out that you've got a potential buffer overflow. Since it's not on the stack, it isn't easily exploitable and may not be a problem.

Code: [Select]
global _main
extern _printf
extern _scanf

section .data
message: db "Hello World!", 0xA, 0x0
; formatin: db "%s\n", 0xA, 0x0
formatin: db "%s", 0x0

section .bss
input: resb 32

section .text
_main:
; push dword [input]
push input
push formatin
call _scanf
        add esp, 4 * 2  ; clean up stack!
; now you can safely "ret"
ret
That's untested, but I "think" it'll work. Let us know if any further problems...

Best,
Frank

Actually I have a follow up question. As regards cleaning the stack when using cdecl functions, does the cleanup occur every time the function is called? So for example:

Code: [Select]
_start:
    push message ; assume message is simply a message to be displayed
    call _printf
    add eps, 4 ; clean up stack (or use pop eax)

    push message
    call _printf
    add eps, 4 ; clean up stack (or use pop eax)

    ret

Would that be the standard way to do it? Also if I was to use pop eax is it good practice to "zero" the register i.e. xor eax, eax ?

Offline Frank Kotler

  • NASM Developer
  • Hero Member
  • *****
  • Posts: 2667
  • Country: us
Re: Using scanf crashes the program [Windows]
« Reply #4 on: September 03, 2014, 03:33:09 AM »
"esp", not "eps" but that's the right idea. Yes, popping a register will do the same thing. Often, "ecx" is used as a "scratch register" for this purpose (Dr. Paul Carter does it this way in his tutorial). "eax" is used for the return value from a function. In the case of _printf, this isn't very interesting. In other cases it is, so you might not want to "pop eax" to clean up the stack, as a general rule.

You don't really need to clean up the stack after each function call. It can be "deferred" and several calls cleaned up at once. It may not be obvious at first, but "call" and "ret" use the stack to store the return address. "call" pushes the return address - the address of the next instruction after the call - and jumps to the function. When the function gets to "ret" it essentially pops the address off the stack to jump back to. At this point, esp had "better be" pointed to a valid return address, or we're going to crash! (oh yeah, we did!)

"xor" is a good way to zero a register - "mov eax, 0" won't kill ya. "pop eax" would overwrite all of eax, so there's no point in zeroing it first. You might want to zero it before the "ret". Returning zero traditionally indicates "no error". We really don't care, but it's "the right thing to do" to return something meaningful. (and to check return values for error before proceding, when appropriate)

It isn't entirely clear, in your last example, whether "_start:" is called. In Linux, the "_start:" label is not called - the very first thing on the stack is "argc" - so "ret" isn't going to work. I think the entrypoint is always called in Windows. It might be "safer" to exit with _exit (or ExitProcess). In either case "push 0" first (or some other meaningful value) - you don't have to clean up after this one. :)

Best,
Frank


Offline PurityLake

  • Jr. Member
  • *
  • Posts: 4
Re: Using scanf crashes the program [Windows]
« Reply #5 on: September 03, 2014, 12:20:10 PM »
I apologize for using the wrong spelling of the register, I wasn't paying full attention to the spelling at the time.

So in the case of:

Code: [Select]
push someValue
push anotherValue
call aFunction
; do some other stuff

What essentially is happening here is that call implicity pushes the address of the next instruction (push PC + 4) PC being the program counter since I'm not sure what the equivalent in nasm would be. call also moves the stack pointer back by 4 so that it is pointing to the previous pushed values then it jumps to the address of the function called. Then when the function uses ret it assumes the next value to be popped is the address to jump to, so it unconditionally jumps to that address (or tried in my case).

Would I be correct in assuming that?

Also as regards the "_start" label, is there anyway to make the linker set an entry point for the case of Linux as you pointed out?

With argc and argv (I assume you can't have one without the other) one could make use of macros to determine whether there is a need to pop those values off the stack before using ret[\i]. What would be easier would be if there is a way to make Windows do the same, this would eliminate the need for conditionally checking for the need to remove stack values before ret.

Thanks again for the answers, they are really aiding me through my learning of assembly. From just two of your posts I have learned so much.

Offline Rob Neff

  • Forum Moderator
  • Full Member
  • *****
  • Posts: 429
  • Country: us
Re: Using scanf crashes the program [Windows]
« Reply #6 on: September 03, 2014, 01:06:41 PM »
Would I be correct in assuming that?

You're getting close!  Read the following articles which will help you further:

http://en.wikipedia.org/wiki/Call_stack

http://en.wikipedia.org/wiki/Calling_convention

Also as regards the "_start" label, is there anyway to make the linker set an entry point for the case of Linux as you pointed out?

Yep.  Every linker has a default that can be overridden by an argument to the executable.

With argc and argv (I assume you can't have one without the other) one could make use of macros to determine whether there is a need to pop those values off the stack before using ret[\i]. What would be easier would be if there is a way to make Windows do the same, this would eliminate the need for conditionally checking for the need to remove stack values before ret.

You may find the portable, cross-platform NASM-X macro package ( written specifically for Nasm ) useful:

http://forum.nasm.us/index.php?topic=1853.0