Well it’s been a long time since I last wrote any entries here. What, almost a year.. time flies by when you’re having fun 
especially when you’re working a lot.
Just wanted to give a quick heads-up that I will start posting again. And what better way to celebrate that than by expanding on that buffer overflow which SVRT-Bkis found this friday.
The overflow is triggered by an overly long TITLE tag, which is later used by SaveFileAsWithFilter as a suggested file name.
The actual overflow is triggered in file_util_win.cc, here’s a snippet:
std::wstring GetDirectoryFromPath(const std::wstring& path) {
wchar_t path_buffer[MAX_PATH];
wchar_t* file_ptr = NULL;
if (GetFullPathName(path.c_str(), MAX_PATH, path_buffer, &file_ptr) == 0)
The current Proof-of-Concept exploit requires social engineering and manual interaction from the user in order to be triggered.
However, the GetDirectoryFromPath function is also being used elsewhere, and one of those places is in download_manager.cc, here’s a snippet:
void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info) {
DCHECK(info);// Check writability of the suggested path. If we can’t write to it, default
// to the user’s “My Documents” directory. We’ll prompt them in this case.
std::wstring path = file_util::GetDirectoryFromPath(info->suggested_path);
This code is automatically called whenever Chrome initiates a file download. The suggested_path in the DownloadCreateInfo struct can be based on different inputs, such as the URL itself. It can also be based on more specific information which will take precedence, such as the one we can specify in a Content-Disposition HTTP header. Therefor, we can automatically trigger the buffer overflow with the following HTTP headers:
Content-Type: application/x-zip-compressed
Content-Disposition: attachment; filename=”$overflowingstringiscontainedwithinthesequotes$”
With that I will go back to enjoying my Laphroaig
UPDATE:
I reported this to the Chromium project as Issue 1980.
They have confirmed that this is now fixed in Google Chrome 0.2.149.29.
I don’t see a buffer overflow in that function. GetFullPathName won’t write more than MAX_PATH characters, which is the size of the buffer.
The function fails to test the return value of GetFullPathName to see if the buffer was too small, and the documentation doesn’t specify what happens to file_ptr in this case, which means that the string length might be calculated incorrectly. However, as the string length is only passed to the wstring constructor, that can’t cause a buffer overflow either.
Am I missing something?
You’re not missing anything. The overflow itself is triggered in GetDirectoryFromPath when path.c_str() is called, but the actual corruption happens earlier.
Cheers
Thor
Hum the issue is not on the snippet code you copy past.
The diff is here : http://src.chromium.org/viewvc/chrome/branches/chrome_official_branch/src/chrome/common/win_util.cc?r1=1766&r2=1765&pathrev=1766
The code was :
// Initially populated by the file component of ’suggested_name’, this buffer
// will be written into by Windows when the user is done with the dialog box.
wchar_t file_name[MAX_PATH+1];
std::wstring file_part = file_util::GetFilenameFromPath(suggested_name);
memcpy(file_name, file_part.c_str(), (file_part.length()+1) * sizeof(wchar_t));
and becomes:
// The size of the in/out buffer in number of characters we pass to win32
// GetSaveFileName. From MSDN “The buffer must be large enough to store the
// path and file name string or strings, including the terminating NULL
// character. … The buffer should be at least 256 characters long.”.
static const size_t kMaxFilenameSize = MAX_PATH + 1;
// Initially populated by the file component of ’suggested_name’, this buffer
// will be written into by Windows when the user is done with the dialog box.
std::wstring file_part = file_util::GetFilenameFromPath(suggested_name);
wchar_t file_name[kMaxFilenameSize];
base::wclscpy(file_name, file_part.c_str(), kMaxFilenameSize);
file_part length is not trusted anymore.
It is because GetDirectoryFromPath is not restrained to a specific size. If the path is superior to MAX_PATH, GetFullPathName return the number of required chars (not 0) and then the entire string is returned.
I diff’ed the file file_util_win.cc from prior to the code fix to better understand. Trying to learn the code to start patch work. Great catch.