List:Commits« Previous MessageNext Message »
From:Magnus Svensson Date:September 16 2008 9:05am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)
Bug#35987
View as plain text  
Vladislav Vaintroub skrev:
> 
>> -----Original Message-----
>> From: Magnus.Svensson@stripped [mailto:Magnus.Svensson@stripped]
>> Sent: Monday, September 15, 2008 9:22 PM
>> To: Vladislav Vaintroub; magnus 'msvensson' Svensson
>> Cc: commits@stripped
>> Subject: Re: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)
>> Bug#35987
>>
>> +      char *p= strrchr(module_dir,'\\');
>> +      if (!p)
>> +      {
>> +        /*
>> +          Path separator was not found. Not known to happen, if ever
> happens,
>> +          will indicate current directory.
>> +        */
>> +        module_dir[0]= '.';
>> +        p= module_dir + 1;
>> +      } 
>> magnus: still feels like this could potentially go wrong. Will look more
>> tomorrow. But maybe I'm just overly careful?
> 
> My guess is overly careful,  and this is actually good :)
> 
> Maybe it helps to know which cases I have tested before claiming there is no
> possibility to have relative paths in Module32First/Next and that unix path
> separators are translated to Windows.
> 
> I  created a dummy DLL in C:\tools. I created a new standalone application
> that does LoadLibrary and get_symbol_path after it and looked if the output
> looks ok.
> I tried 
> 1) LoadLibrary("mydll.dll") (with PATH previously set to "%PATH%;C:\tools")
> 2) LoadLibrary ("C:\\tools\\mydll.dll")
> 3) LoadLibrary("C:/tools/mydll.dll")
> 
> In all cases, after load library the Module32Next returned
> "C:\tools\mydll.dll" in mod.szExePath.
> 
> I repeated 1) with mydll.dll copied in the same directory as the test
> program , and also after that  mod.szExePath was an absolute path.
> 

Looks good. So you are basically saying that there is no way we can get 
p==NULL after strrchr?

What I was suspicious about is:

1. to modify the data that "module_dir" is pointing to, are we sure it's 
pointing to a a buffer we can modify? But I don't see how MODULEENTRY32 
is defined, so maybe there is a big buffer in there ie that should be safe.

2. to modify two bytes in the string that "p" points to. The last "\" 
could potentially point to the last position in that buffer and thus, 
the second assignment of '\0' would be written after end of buffer.


I would suggest:
       char *module_dir= mod.szExePath;
       char *p= strrchr(module_dir,'\\');
       if (p)
         *p= '\0'; // Truncate string at last \

       if (!strstr(path, module_dir))
       {
         strncat(path, module_dir, size);
         strcat(path, ";");
         size-= strlen(module_dir)+1; // << From 3) below
       }

3. I also just remembered that the size argument for strncat actually 
means how many chars to take from the src string, thus it's .actually 
not a way of preventing us from writing beyond end of "path".

So you probably need to subtract the size of the "module_dir" after 
having appended it. Then use a really small size of "path" and try again  :)



Thread
bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678) Bug#35987Vladislav Vaintroub15 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)Bug#35987Magnus Svensson15 Sep
    • RE: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)Bug#35987Vladislav Vaintroub15 Sep
      • Re: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)Bug#35987Magnus Svensson16 Sep
        • RE: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)Bug#35987Vladislav Vaintroub16 Sep
          • Re: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)Bug#35987Magnus Svensson16 Sep