This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Crash when reading "import table" of certain windows DLLs
Type: Stage:
Components: Windows Versions: Python 2.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: theller Nosy List: mhammond, theller, tim.peters
Priority: high Keywords:

Created on 2004-05-11 11:02 by mhammond, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
dumpbin.txt theller, 2004-05-11 15:45 Output of 'dumpbin.exe /all \windows\system32\wmi.dll'
wmi_crash.patch mhammond, 2004-05-12 01:56 New patch
dynload_win.c-1.patch theller, 2004-06-01 17:45 dynload_win.c-1.patch
dynload_win.c-2.patch theller, 2004-06-01 17:46 dynload_win.c-2.patch
Messages (15)
msg20759 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2004-05-11 11:02
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.
msg20760 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2004-05-11 11:05
Logged In: YES 
user_id=14198

Actually, I guess a comment regarding the pointer checks and
referencing this bug would be nice :)
msg20761 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-05-11 14:49
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.
msg20762 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2004-05-11 15:20
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.
msg20763 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-05-11 15:42
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.
msg20764 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-05-11 15:45
Logged In: YES 
user_id=11105

Oh, we have to give the /all option to dumpbin ;-)
msg20765 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2004-05-11 23:00
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.
msg20766 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2004-05-12 01:56
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.
msg20767 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-06-01 17:45
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).
msg20768 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-06-01 18:35
Logged In: YES 
user_id=11105

This is not yet accepted.
msg20769 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-06-28 21:28
Logged In: YES 
user_id=11105

It seems Mark doesn't listen (or don't have time).
I'd like to check this in for 2.4.  Any objections?
msg20770 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2004-06-28 23:20
Logged In: YES 
user_id=14198

I'm sorry, but I'm not sure what I don't listen to :)  You
are correct about being short of time though.  What would
you like me to do?
msg20771 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-06-29 19:46
Logged In: YES 
user_id=11105

Sorry if I was confusing.
I simply want to know if this should be checked in or not. 
Maybe you can review the code, and/or try it out.  Thanks.
msg20772 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2004-07-01 00:07
Logged In: YES 
user_id=14198

Sorry about that - I thought my "IsBadReadPtr()" patch did
end up getting checked in :(  It is still sitting as a diff
in my 2.3 tree, which is a shame.  Sorry about that.

You patch works fine with the 2.4 branch, and does also stop
the crash, so please check it in.
msg20773 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2004-07-02 08:54
Logged In: YES 
user_id=11105

Commited as dynload_win.c rev 2.12.14.1 and rev 2.13.
History
Date User Action Args
2022-04-11 14:56:04adminsetgithub: 40243
2004-05-11 11:02:42mhammondcreate