Export to GitHub

miranda - issue #1742

Dangerous call to MultiByteToWideChar


Posted on Nov 22, 2013 by Swift Bear

[I am unaware of any actual effect that the following report has, but by my reading and by defensive coding standards (the difference between "obviously having no errors" and "having no obvious errors") this is an error.]

src/modules/srfile/fileexistsdlg.c contains (lines 60 and 62)

WCHAR wszFilename[MAX_PATH]; MultiByteToWideChar(CP_ACP, 0, szFilename, -1, wszFilename, sizeof(wszFilename));

The final parameter of MultiByteToWideChar is supposed to be the number of CHARACTERS that are in wszFilename, not the number of bytes, so the buffer is half the size of what you're saying it is. I believe it should be MAX_PATH (or perhaps sizeof(wszFilename)/sizeof(wszFilename[0]) if you prefer that).

See http://msdn.microsoft.com/en-us/library/windows/desktop/dd319072(v=vs.85).aspx (In fact there's a big caution at the top warning about this misuse)

Now, it may well be that the filename being converted can never be longer than MAX_PATH, but if that ever comes from a source where the length isn't checked, the given call can overrun the buffer. In addition, it is possible to have paths longer than MAX_PATH in Windows by using "\?\" paths (see http://msdn.microsoft.com/en-us/library/aa365247(VS.85).aspx#maxpath), which means that even if a filename successfully is parsed by some Windows file functions may not guarantee that it is short enough.

Comment #1

Posted on Dec 18, 2013 by Helpful Dog

Fixed in svn.

Comment #2

Posted on Dec 18, 2013 by Helpful Dog

(No comment was entered for this change.)

Status: Fixed

Labels:
Private Priority-High Type-Bug