Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False uninits in ADVAPI32.dll!SystemFunction036 #65

Closed
derekbruening opened this issue Nov 28, 2014 · 12 comments
Closed

False uninits in ADVAPI32.dll!SystemFunction036 #65

derekbruening opened this issue Nov 28, 2014 · 12 comments

Comments

@derekbruening
Copy link
Contributor

From timurrrr@google.com on October 06, 2010 09:30:43

As of r65 , the following code gives 2 uninits below SystemFunction036:
===test.cpp===
#include <windows.h>

typedef BOOL (WINAPI *SystemFunction036_T)(PVOID, ULONG);
SystemFunction036_T SystemFunction036;

int main() {
char tmp[10];
HINSTANCE advapi32 = LoadLibraryA("advapi32.dll");
SystemFunction036 = (SystemFunction036_T)GetProcAddress(advapi32, "SystemFunction036");
(*SystemFunction036)(tmp, 10);
return 0;
}

->
Error #1: UNINITIALIZED READ: reading 0x0012ff10-0x0012ff14 4 byte(s)
@0:00:04.513 in thread 3220
0x00aa8315 <ADVAPI32.dll+0x8315> ADVAPI32.dll!SystemFunction036
0x00aa82f3 <ADVAPI32.dll+0x82f3> ADVAPI32.dll!SystemFunction036
0x00aa82b6 <ADVAPI32.dll+0x82b6> ADVAPI32.dll!SystemFunction036
0x0040104e <test.exe+0x104e> test.exe!main z:\dr-sandbox\issues\systemfunction036\test.cpp:10

I really enjoyed the name of the function and the way libjingle uses it http://google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/third_party/libjingle/source/talk/base/helpers.cc&q=source/talk/base/helpers.cc&l=66 looks like a hack...

But I also saw such reports in other advapi32.dll functions when they are used correctly.

According to http://source.winehq.org/WineAPI/SystemFunction036.html , this function invokes RtlGenRandom

Original issue: http://code.google.com/p/drmemory/issues/detail?id=65

@derekbruening
Copy link
Contributor Author

From rsleevi@chromium.org on May 24, 2011 20:29:03

For what it's worth, it's correct in reporting an uninitialized read - but that is an edge case.

In the above example, SystemFunction036 will read from tmp. This is because it uses the buffer supplies as an auxiliary random seed.

CryptGenRandom, which is just a facade for SystemFunction036 in the default MSFT provider, documents this (since "technically" SystemFunction036 is undocumented/unsupported) http://msdn.microsoft.com/en-us/library/aa379942(VS.85).aspx As a way to confirm this, if you initialize tmp (eg: memset), does the error disappear?

I would agree that there is a desired change here, even if it's not a FalsePositive, since it would be better to suppress this error universally, lest users end up thinking similarly to http://digitaloffense.net/tools/debian-openssl/ . While CryptGenRandom/SystemFunction036 are cryptographically strong by themselves (meaning the additional entropy is not necessary), it would seem better not to bubble this up at all.

@derekbruening
Copy link
Contributor Author

From timurrrr@google.com on June 21, 2011 03:19:32

as of r343 , the report looks different (this is Win XP 32-bit):
Error #1: UNINITIALIZED READ: reading 0x77e46318-0x77e46328 16 byte(s) within 0x77e46318-0x77e46418
@0:00:00.844 in thread 3884
system call NtDeviceIoControlFile InputBuffer

0x77dd9559 <ADVAPI32.dll+0x9559> ADVAPI32.dll!MD4Update
0x77dd8453 <ADVAPI32.dll+0x8453> ADVAPI32.dll!SystemFunction036
0x77dd8388 <ADVAPI32.dll+0x8388> ADVAPI32.dll!SystemFunction036
0x77dd834b <ADVAPI32.dll+0x834b> ADVAPI32.dll!SystemFunction036
0x77dd82f3 <ADVAPI32.dll+0x82f3> ADVAPI32.dll!SystemFunction036
0x77dd82b6 <ADVAPI32.dll+0x82b6> ADVAPI32.dll!SystemFunction036
0x0040104e <test.exe+0x104e> test.exe!main
c:\sandbox\sysf036\test.cpp:10

Probably the difference is caused by a new ioctl handling code.

Ryan,
Yes, the uninit report goes away if I add "memset(tmp, 0, 10);" before the (*SystemFunction036) line.

@derekbruening
Copy link
Contributor Author

From timurrrr@google.com on June 21, 2011 03:28:05

Marking this as WontFix.

Ryan, thanks for the info!

Status: WontFix
Labels: -Bug-FalsePositive ThirdParty-Bug

@derekbruening
Copy link
Contributor Author

From timurrrr@google.com on June 21, 2011 03:28:56

Issue 15 has been merged into this issue.

Cc: timurrrr@google.com

@derekbruening
Copy link
Contributor Author

From timurrrr@google.com on July 20, 2011 07:34:00

Derek,
Shall I add a suppression like this to the default list?

UNINITIALIZED READ
...
*!SystemFunction036

Status: Started
Cc: -timurrrr@google.com bruen...@google.com

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on July 20, 2011 07:43:17

I don't follow -- did something happen to trigger re-opening this issue?
there are several dlls with a "SystemFunction036": what makes you think they're related?

@derekbruening
Copy link
Contributor Author

From timurrrr@google.com on July 20, 2011 07:46:39

I don't follow -- did something happen to trigger re-opening this issue?
I was just revisiting the Chromium suppression file.

there are several dlls with a "SystemFunction036"
UNINITIALIZED READ
...
ADVAPI32.dll!SystemFunction036
?

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on July 20, 2011 13:28:38

so this is still "SystemFunction036" (the chrome instances) even w/ symbols? what's the caller there?

seems to call advapi32!NewGenRandom on xpwow64.

if it really is always a crypto func on all platforms w/ its buf param as OUT only, suppressing uninits seems reasonable. though I wonder how many nearby non-exported functions show up as this (though that applies to other suppressions too).

@derekbruening
Copy link
Contributor Author

From timurrrr@google.com on July 22, 2011 08:06:33

No surprise SystemFunction036 keeps its name even with symbols since this is an exported symbol.

Proof:
[code taken from issue #18 ]

#include <windows.h>
#include <Objbase.h>
#include <stdio.h>

#pragma comment(lib, "Ole32.lib")

int main() {
CoInitialize(NULL);
CoUninitialize();
printf("PASS\n");
}

[XP 32-bit report with symbols]
Error #1: UNINITIALIZED READ: reading 0x77e4631f-0x77e4632f 16 byte(s) within 0x77e46318-0x77e46418
@0:00:01.282 in thread 4524
system call NtDeviceIoControlFile InputBuffer

0x77dd9559 <ADVAPI32.dll+0x9559> ADVAPI32.dll!GatherRandomKeyFastUserMode
0x77dd8453 <ADVAPI32.dll+0x8453> ADVAPI32.dll!RandomFillBuffer
0x77dd8388 <ADVAPI32.dll+0x8388> ADVAPI32.dll!GenRandom
0x77dd834b <ADVAPI32.dll+0x834b> ADVAPI32.dll!NewGenRandomEx
0x77dd82f3 <ADVAPI32.dll+0x82f3> ADVAPI32.dll!NewGenRandom
0x77dd82b6 <ADVAPI32.dll+0x82b6> ADVAPI32.dll!SystemFunction036 # Yes! It's still SystemFunction036
0x77e762f8 <RPCRT4.dll+0x62f8> RPCRT4.dll!GenerateRandomNumber
0x77e76265 <RPCRT4.dll+0x6265> RPCRT4.dll!UuidCreate
0x77501459 <ole32.dll+0x21459> ole32.dll!wCoCreateGuid
0x775018d2 <ole32.dll+0x218d2> ole32.dll!CObjectContext::CreateObjectContext
0x77501691 <ole32.dll+0x21691> ole32.dll!InitThreadCtx
0x775015fb <ole32.dll+0x215fb> ole32.dll!wCoInitializeEx
0x77501539 <ole32.dll+0x21539> ole32.dll!CoInitializeEx
0x7752f959 <ole32.dll+0x4f959> ole32.dll!CoInitialize
0x0040101b <test.exe+0x101b> test.exe!main
c:\sandbox\18\test.cpp:8

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on July 22, 2011 08:14:47

No surprise SystemFunction036 keeps its name even with symbols since this is an exported symbol.

It has to be an exported symbol to show up w/o symbols, but w/o +offs ( issue #290 ) it could easily be one of dozens of other functions in reality

@derekbruening
Copy link
Contributor Author

From timurrrr@google.com on July 22, 2011 11:36:35

The uninits were suppressed in r412

Status: Fixed

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on August 02, 2011 13:09:30

xref issue #503 comment 7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant