Zeroes buffer for DocumentProperti

printing/backend/win_helper.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "printing/backend/win_helper.h"
 
 #include <algorithm>
 
 #include "base/file_version_info.h"
 #include "base/files/file_path.h"
@@ skipping 460 matching lines @@
 }
 
 scoped_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
                                                      DEVMODE* in) {
   LONG buffer_size = DocumentProperties(
       NULL, printer, const_cast<wchar_t*>(L""), NULL, NULL, 0);
   if (buffer_size < static_cast<int>(sizeof(DEVMODE)))
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
   scoped_ptr<DEVMODE, base::FreeDeleter> out(
       reinterpret_cast<DEVMODE*>(malloc(buffer_size)));
+  memset(out.get(), 0, buffer_size);

[Lei Zhang, 2014/10/08 20:11:34]

Just calloc() instead?

[Vitaly Buka (NO REVIEWS), 2014/10/08 20:18:15]

Done.

   DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER;
   if (DocumentProperties(
           NULL, printer, const_cast<wchar_t*>(L""), out.get(), in, flags) !=
       IDOK) {
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
   }
-  CHECK_GE(buffer_size, out.get()->dmSize + out.get()->dmDriverExtra);
+  WORD size = out->dmSize;
+  WORD extra_size = out->dmDriverExtra;
+  CHECK_GE(buffer_size, size + extra_size);

[Lei Zhang, 2014/10/08 20:11:34]

Do we care if size + extra_size overflows?

[Vitaly Buka (NO REVIEWS), 2014/10/08 20:18:15]

Done.

[Lei Zhang, 2014/10/08 20:24:15]

I just checked and a WORD is only 16-bit. There's no way 2 WORDS added together
can overflow a LONG. So why not just: LONG actual_size = out->dmSize +
out->dmDriverExtra; ?

[Vitaly Buka (NO REVIEWS), 2014/10/08 20:32:55]

C++ does WORD+WORD->WORD, so old version indeed had overflow problem.
However it made no difference because overflow would likely satisfy CHECK_GE

LONG would work instead of int, but we don't need LONG for api calls like above,
so int should be used, according our style.

Also I put two local variable hoping to see them in future crash dumps on stack.


On 2014/10/08 20:24:15, Lei Zhang wrote:

   return out.Pass();
 }
 
 scoped_ptr<DEVMODE, base::FreeDeleter> PromptDevMode(
     HANDLE printer,
     const base::string16& printer_name,
     DEVMODE* in,
     HWND window,
     bool* canceled) {
   LONG buffer_size =
       DocumentProperties(window,
                          printer,
                          const_cast<wchar_t*>(printer_name.c_str()),
                          NULL,
                          NULL,
                          0);
   if (buffer_size < static_cast<int>(sizeof(DEVMODE)))
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
   scoped_ptr<DEVMODE, base::FreeDeleter> out(
       reinterpret_cast<DEVMODE*>(malloc(buffer_size)));
+  memset(out.get(), 0, buffer_size);
   DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER | DM_IN_PROMPT;
   LONG result = DocumentProperties(window,
                                    printer,
                                    const_cast<wchar_t*>(printer_name.c_str()),
                                    out.get(),
                                    in,
                                    flags);
   if (canceled)
     *canceled = (result == IDCANCEL);
   if (result != IDOK)
     return scoped_ptr<DEVMODE, base::FreeDeleter>();
-  CHECK_GE(buffer_size, out.get()->dmSize + out.get()->dmDriverExtra);
+  WORD size = out->dmSize;
+  WORD extra_size = out->dmDriverExtra;
+  CHECK_GE(buffer_size, size + extra_size);
   return out.Pass();
 }
 
 }  // namespace printing