[ python-Bugs-951851 ] Crash when reading "import table" of certain windows DLLs

Tue Jun 1 14:35:19 EDT 2004

Bugs item #951851, was opened at 2004-05-11 13:02
Message generated for change (Comment added) made by theller
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=951851&group_id=5470

Category: Windows
Group: Python 2.3
Status: Open
>Resolution: None
Priority: 7
Submitted By: Mark Hammond (mhammond)
Assigned to: Mark Hammond (mhammond)
Summary: Crash when reading "import table" of certain windows DLLs

Initial Comment:
As diagnosed by Thomas Heller, via the python-win32 list.

On Windows 2000, if your sys.path includes the Windows
system32 directory, 'import wmi' will crash Python.  To
reproduce, change to the system32 directory, start
Python, and execute 'import wmi'.  Note that Windows XP
does not crash.

The problem is in GetPythonImport(), in code that tries
to walk the PE 'import table'.  AFAIK, this is code
that checks the correct Python version is used, but
I've never seen this code before.

I'm not sure why the code is actually crashing (ie,
what assumptions made about the import table are
wrong), but I have a patch that checks a the pointers
are valid before they are de-referenced.  After the
patch is applied, the result is the correct:
"ImportError: dynamic module does not define init
function (initwmi)"
exception.

Assigning to thomas for his review, then I'm happy to
check in.  I propose this as a 2.3 bugfix.

----------------------------------------------------------------------

>Comment By: Thomas Heller (theller)
Date: 2004-06-01 20:35

Message:
Logged In: YES 
user_id=11105

This is not yet accepted.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2004-06-01 19:45

Message:
Logged In: YES 
user_id=11105

The reason the current code crashed when Python tries to
import Win2k's or XP's wmi.dll as extension is that the size
of the import table in this dll is zero.  The first patch
'dynload_win.c-1.patch' fixes this by returning NULL in that
case.

The code, however, doesn't do what is intended in a debug
build of Python.  It looks for imports of 'python23.dll',
when it should look for 'python23_d.dll' instead.  The
second patch 'dynload_win.c-2.patch' fixes this also (and
includes the first patch as well).

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2004-05-12 03:56

Message:
Logged In: YES 
user_id=14198

Seeing as it was the de-referencing of 'import_name' that
crashed, I think a better patch is to terminate the outer
while look as soon as we hit a bad string.  Otherwise, this
outer loop continues, 20 bytes at a time, until the outer
pointer ('import_data') also goes bad or happens to
reference \0.

Attaching a slightly different patch, with comments and
sizeof() change.

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2004-05-12 01:00

Message:
Logged In: YES 
user_id=14198

OK - will change to 12+so(WORD)

And yes, I had seen this code - I meant "familiar with" :)

Tim: Note that the import failure is not due to a bad import
table (but the crash was).  This code is trying to determine
if a different version of Python is used.  We are
effectively skipping that check, and landing directly in the
"does it have an init function?", then faling normally - ie,
the code is now correctly *not* finding other Python
versions linked against it.  Thus, a different error message
doesn't make much sense to me.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2004-05-11 17:45

Message:
Logged In: YES 
user_id=11105

Oh, we have to give the /all option to dumpbin ;-)

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2004-05-11 17:42

Message:
Logged In: YES 
user_id=11105

Tim, I don't think the import table format has changed,
instead wmi.dll doesn't have an import table (for whatever
reason).  Maybe the  code isn't able to handle that correctly.

Since Python 2.3 as well at it's extensions are still built
with MSVC 6, I *think* we should be safe with this code.

I'll attach the output of running MSVC.NET 2003's
'dumpbin.exe \windows\system32\wmi.dll' on my WinXP Pro SP1
for the curious.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2004-05-11 17:20

Message:
Logged In: YES 
user_id=31435

Mark, while you may not have seen this code before, you 
checked it in <wink -- rev 2.7>.  IIRC, though, the person 
who *created* the patch was never heard from again.

I don't understand what the code thinks it's doing either, 
exactly.  The obvious concern:  if the import table format has 
changed, won't we also fail to import legit C extensions?  I 
haven't seen that happen yet, but I haven't yet built any 
extensions using VC 7.1 either.

In any case, I'd agree it's better to get a mysterious import 
failure than a crash.  Maybe the detail in the ImportError 
could be changed to indicate whan an import failure is due to 
a bad pointer.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2004-05-11 16:49

Message:
Logged In: YES 
user_id=11105

IMO, IsBadReadPointer(import_data, 12 + sizeof(DWORD))
should be enough.

Yes, please add a comment in the code.

This is a little bit hackish, but it fixes the problem.  And
the real problem can always be fixed later, if needed.

And, BTW, python 2.3.3 crashes on Windows XP as well.

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2004-05-11 13:05

Message:
Logged In: YES 
user_id=14198

Actually, I guess a comment regarding the pointer checks and
referencing this bug would be nice :)

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=951851&group_id=5470