Sign in to follow this  
Followers 0
JohnOne

C++ dll woes (solved)

27 posts in this topic

#1 ·  Posted (edited)

Here is the code in dll

LPWSTR __stdcall tmppath()
{
LPWSTR path[MAX_PATH];
if(GetTempPath(MAX_PATH, *path)){
  return *path;
}
else{
  return L"0";
}
Here Autoit code

$adll = DllCall("testdll.dll", "wstr", "tmppath")
If Not @error Then
MsgBox(0, "Return", VarGetType($adll[0]) & " " & $adll[0] & " Length " & StringLen($adll[0]))
Else
MsgBox(0, "Error", @error)
EndIf

The problem is that sometimes it crashes Autoit, and sometimes it returns my temp dir.

What am I doing wrong here? (besides trying to code c++)

Edited by JohnOne

AutoIt Absolute Beginners    Require a serial    Pause Script    Video Tutorials by Morthawt   ipify 

Monkey's are, like, natures humans.

Share this post


Link to post
Share on other sites



#4 ·  Posted (edited)

When the function returns "path" is deallocated. Sometimes that memory is cleared (crash). Sometimes it will be left alone for a while and that's when it "seems" to work. You could allocate the memory area on DllOpen as a global and use that. Or pass it a string as a parameter and reuse that rather than trying to do it as a return type.

Edited by Jon

Share this post


Link to post
Share on other sites

#6 ·  Posted (edited)

I cant get my head around how to do that :)

This is the latest attempt.

extern "C" int __stdcall tmppath(LPWSTR[MAX_PATH]);
int __stdcall tmppath(LPWSTR rtn)
{
if(GetTempPath(MAX_PATH, rtn)){
  return 1;
}
else{
  return 0;
}

$hDll = DllOpen("codedll.dll")
Local $struct = "char[260]"
Local $dllstruct = DllStructCreate($struct)
If @error Then Exit
$adll = DllCall($hDll, "int", "tmppath","wstr",$dllstruct) ; tried wstr*, wstr, struct, struct*, (struct* + struct crashes autoit)
If Not @error Then
$rtn  = DllStructGetData($dllstruct,1)
MsgBox(0, "Return", VarGetType($rtn) & " " & $rtn & " Length " & StringLen($rtn))
Else
MsgBox(0, "Error", @error)
EndIf
DllClose($hDll)

Output is "String Length 0"

Edited by JohnOne

AutoIt Absolute Beginners    Require a serial    Pause Script    Video Tutorials by Morthawt   ipify 

Monkey's are, like, natures humans.

Share this post


Link to post
Share on other sites

I think your declaration should be

extern "C" int __stdcall tmppath(LPWSTR);

And autoit

$adll = DllCall($hDll, "int", "tmppath", "wstr", "")
If Not @error Then
    MsgBox(0, "Return", $adll[1] & " " & $adll[1] & " Length " & StringLen($adll[1]))
Else
    MsgBox(0, "Error", @error)
EndIf

Share this post


Link to post
Share on other sites

I'd recommend passing both a buffer and a length as parameters, to avoid buffer overruns.

Share this post


Link to post
Share on other sites

I'm not sure what that means, I thought MAX_PATH was the largest size you could have.

I wouldn't know what size the the actual string is to get it precise enough to pass in.

Of course I'm probably misunderstanding your tip.


AutoIt Absolute Beginners    Require a serial    Pause Script    Video Tutorials by Morthawt   ipify 

Monkey's are, like, natures humans.

Share this post


Link to post
Share on other sites

Actually file names can be longer than MAX_PATH when using certain extended versions of Windows functions. But that wasn't the point of my tip.

The buffer that is allocated in AutoIt is something like char[x] where x is a number. You need to tell your function what x is so that the function doesn't write more data than that.

function(buffer, x)

And then you can't write more than x characters inside the function. If you do, you either throw a runtime error (in checked environments) or screw up potentially sensitive parts of the calling program.

Share this post


Link to post
Share on other sites

Buffer overruns are an easy mistake to make, especially for a new programmer. But they are evil. On a good day a buffer overrun will just be a crash. On a bad day you'll leave a buffer overrun in an elevated process and it will be trivial to find and exploit to execute arbitrary code (elevated).

Share this post


Link to post
Share on other sites

#14 ·  Posted (edited)

I'd recommend passing both a buffer and a length as parameters, to avoid buffer overruns.

In this implementation it's not necessary. The AutoIt wstr type is some huge size, in fact I'm not even sure it can overrun (devs?). Additionally he's already telling GetTempPath that MAX_PATH is the size of AutoIt's string buffer, which is fine. I mean sure, you could go all out and define your own buffer with DllStructCreate and pass extra params to the DLL function, and I'm not arguing that isn't the best way to do it from a coding standpoint, but is that really necessary here? I don't think so. That's the whole point of the wstr type.

Grumble. I almost didn't write this because I expected Valik's response. The brain-finger filter learns every day...

Edited by wraithdu

Share this post


Link to post
Share on other sites

I wonder how many exploits have been created because the author of a piece of code made a conscious choice to ignore safety because "it's not really necessary here"? If for no other reason then a trivial function where it isn't necessary is the best place to learn to write functions correctly since the negative side effects are minimal if it's done wrong.

Share this post


Link to post
Share on other sites

#16 ·  Posted (edited)

To satisfy my curiosity, is the AutoIt wstr type exploitable? Can it be overrun, or will it resize indefinitely (within the limits of available memory), or eventually just crash the script?

Edited by wraithdu

Share this post


Link to post
Share on other sites

For input it will be 65536 characters minimum but expand beyond that if you pass in a very large string. However, it is possible the callee could write to the buffer and overrun it.

Share this post


Link to post
Share on other sites

Do you think it would be wise to define a MAX_SIZE in the c++ code instead of using MAX_PATH?

after all, in this case it's only after the @TempDIR, and I suspect it would be unusual for that to

be a massive size.


AutoIt Absolute Beginners    Require a serial    Pause Script    Video Tutorials by Morthawt   ipify 

Monkey's are, like, natures humans.

Share this post


Link to post
Share on other sites

You shouldn't use hard coded sizes when receiving buffers from other contexts.

Share this post


Link to post
Share on other sites

I wonder how many exploits have been created because the author of a piece of code made a conscious choice to ignore safety because "it's not really necessary here"? If for no other reason then a trivial function where it isn't necessary is the best place to learn to write functions correctly since the negative side effects are minimal if it's done wrong.

I was going to bring up the original xbox game save exploits, but after some reading I found those were cause by buffer underrun, not overrun. Two different worlds apparently.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0