Buffer overflow or false positive?

Question

getmodulefilenamew function produces false positive (buffer overflow) as it accepts second argument as buffer - of fixed size in our case.

But looking through its documentation: http://msdn.microsoft.com/en-us/library/ms683197%28v=vs.85%29.aspx

Quote: If the buffer is too small to hold the module name, the string is truncated to nSize characters including the terminating null character, the function returns nSize, and the function sets the last error to ERROR_INSUFFICIENT_BUFFER.

Can somebody as trusted third party person confirm or reject this issue as false positive. Thanks for your help!

===

HMODULE applicationModule = GetModuleHandleW(NULL);
WCHAR processName[MAX_PATH];
memset(processName, 0, sizeof(processName));
GetModuleFileNameW(applicationModule, processName, sizeof(processName));

===

The problem is line with GetModuleFileNameW function

Scan was provided by Veracode static analyzer.

Are you talking about some kind of static checking tool? Tell us which, and show us your code. Otherwise no chance of a meaningful answer. — hmakholm left over Monica, Aug 10 '11 at 21:04

score 2 · Accepted Answer · answered Aug 10 '11 at 21:12

2

Your problem is that you are passing an incorrect value for nSize. You are passing the number of bytes but you should be passing the number of characters, MAX_PATH. These values differ because a wide character has a size of 2 bytes.

So, yes there is an error in your code. If the module name is sufficiently long, Windows will attempt to write up to 520 characters to a buffer that only has room for 260.

answered Aug 10 '11 at 21:12

David Heffernan

601,492
42
1,072
1,490

I understand. The buffer overflow is not possible in this case, because buffer will simply be truncated to the max available size + null terminating character. Just need somebody to comment/approve on this. Sorry for bothering with such question. – Is this issue a false positive Aug 10 '11 at 21:14
No, the buffer overflow is very possible. The buffer is smaller than you are saying it is. For a module name longer than 260 characters, you will have a buffer overrun. – David Heffernan Aug 10 '11 at 21:16
But: "If the module name is larger than the provided buffer then the function will write nSize characters to the buffer, truncating the remaining characters." Yes, in such case I will not get full name (260+) but will get truncated name instead. Did I understand this documentation correct? – Is this issue a false positive Aug 10 '11 at 21:18
What you don't understand is that you are **passing an incorrect value of `nSize`**. `nSize` is the number of **characters** in the buffer. You are passing the number of **bytes**. These quantities differ by a factor of 2 since you are using wide characters. So you have a potential buffer overflow which occurs whenever there is a module with a name longer than 260 characters. – David Heffernan Aug 10 '11 at 21:22
To make it really concrete, you pass a value for `nSize` of 520. Windows will therefore write up to 520 characters to the buffer. The buffer only has room for 260 characters. Boom! – David Heffernan Aug 10 '11 at 21:23
1

Thanks a lot, David Heffernan! – Is this issue a false positive Aug 10 '11 at 21:25

Buffer overflow or false positive?

1 Answers1