diff options
37 files changed, 214 insertions, 271 deletions
diff --git a/base/base_paths_mac.mm b/base/base_paths_mac.mm index a9cd471b..aed3ea8 100644 --- a/base/base_paths_mac.mm +++ b/base/base_paths_mac.mm @@ -18,24 +18,18 @@ namespace { -bool GetNSExecutablePath(FilePath* path) WARN_UNUSED_RESULT; - -bool GetNSExecutablePath(FilePath* path) { +void GetNSExecutablePath(FilePath* path) { DCHECK(path); // Executable path can have relative references ("..") depending on // how the app was launched. uint32_t executable_length = 0; _NSGetExecutablePath(NULL, &executable_length); - DCHECK_GE(executable_length, 1u); + DCHECK_GT(executable_length, 1u); std::string executable_path; - char* executable_path_c = WriteInto(&executable_path, executable_length); - int rv = _NSGetExecutablePath(executable_path_c, &executable_length); + int rv = _NSGetExecutablePath(WriteInto(&executable_path, executable_length), + &executable_length); DCHECK_EQ(rv, 0); - DCHECK(!executable_path.empty()); - if ((rv != 0) || (executable_path.empty())) - return false; *path = FilePath(executable_path); - return true; } // Returns true if the module for |address| is found. |path| will contain @@ -58,7 +52,8 @@ namespace base { bool PathProviderMac(int key, FilePath* result) { switch (key) { case base::FILE_EXE: - return GetNSExecutablePath(result); + GetNSExecutablePath(result); + return true; case base::FILE_MODULE: return GetModulePathForAddress(result, reinterpret_cast<const void*>(&base::PathProviderMac)); diff --git a/base/rand_util.cc b/base/rand_util.cc index fcbccef..a9bc961 100644 --- a/base/rand_util.cc +++ b/base/rand_util.cc @@ -71,6 +71,7 @@ void RandBytes(void* output, size_t output_length) { } std::string RandBytesAsString(size_t length) { + DCHECK_GT(length, 0u); std::string result; RandBytes(WriteInto(&result, length + 1), length); return result; diff --git a/base/rand_util.h b/base/rand_util.h index 4474c02..8d795a3 100644 --- a/base/rand_util.h +++ b/base/rand_util.h @@ -38,9 +38,9 @@ BASE_EXPORT double BitsToOpenEndedUnitInterval(uint64 bits); BASE_EXPORT void RandBytes(void* output, size_t output_length); // Fills a string of length |length| with with cryptographically strong random -// data and returns it. +// data and returns it. |length| should be nonzero. // -// Not that this is a variation of |RandBytes| with a different return type. +// Note that this is a variation of |RandBytes| with a different return type. BASE_EXPORT std::string RandBytesAsString(size_t length); } // namespace base diff --git a/base/rand_util_unittest.cc b/base/rand_util_unittest.cc index a3474ba..e0e85ec 100644 --- a/base/rand_util_unittest.cc +++ b/base/rand_util_unittest.cc @@ -41,8 +41,8 @@ TEST(RandUtilTest, RandBytes) { } TEST(RandUtilTest, RandBytesAsString) { - std::string random_string = base::RandBytesAsString(0); - EXPECT_EQ(0U, random_string.size()); + std::string random_string = base::RandBytesAsString(1); + EXPECT_EQ(1U, random_string.size()); random_string = base::RandBytesAsString(145); EXPECT_EQ(145U, random_string.size()); char accumulator = 0; diff --git a/base/string_util.h b/base/string_util.h index c359e73..a135180 100644 --- a/base/string_util.h +++ b/base/string_util.h @@ -457,40 +457,32 @@ BASE_EXPORT void ReplaceSubstringsAfterOffset( const std::string& find_this, const std::string& replace_with); -// Reserves enough memory in |str| to accommodate |length_with_null| -// characters, sets the size of |str| to |length_with_null - 1| characters, -// and returns a pointer to the underlying contiguous array of characters. +// Reserves enough memory in |str| to accommodate |length_with_null| characters, +// sets the size of |str| to |length_with_null - 1| characters, and returns a +// pointer to the underlying contiguous array of characters. This is typically +// used when calling a function that writes results into a character array, but +// the caller wants the data to be managed by a string-like object. It is +// convenient in that is can be used inline in the call, and fast in that it +// avoids copying the results of the call from a char* into a string. // -// This is typically used when calling a function that writes results into a -// character array, but the caller wants the data to be managed by a -// string-like object. +// |length_with_null| must be at least 2, since otherwise the underlying string +// would have size 0, and trying to access &((*str)[0]) in that case can result +// in a number of problems. // -// |length_with_null| must be >= 1. In the |length_with_null| == 1 case, -// NULL is returned rather than a pointer to the array, since there is no way -// to provide access to the underlying character array of a 0-length -// string-like object without breaking guarantees provided by the C++ -// standards. -// -// Internally, this takes linear time because the underlying array needs to -// be 0-filled for all |length_with_null - 1| * sizeof(character) bytes. +// Internally, this takes linear time because the resize() call 0-fills the +// underlying array for potentially all +// (|length_with_null - 1| * sizeof(string_type::value_type)) bytes. Ideally we +// could avoid this aspect of the resize() call, as we expect the caller to +// immediately write over this memory, but there is no other way to set the size +// of the string, and not doing that will mean people who access |str| rather +// than str.c_str() will get back a string of whatever size |str| had on entry +// to this function (probably 0). template <class string_type> inline typename string_type::value_type* WriteInto(string_type* str, size_t length_with_null) { - DCHECK_NE(0u, length_with_null); + DCHECK_GT(length_with_null, 1u); str->reserve(length_with_null); str->resize(length_with_null - 1); - - // If |length_with_null| is 1, calling (*str)[0] is invalid since the - // size() is 0. In some implementations this triggers an assertion. - // - // Trying to access the underlying byte array by casting away const - // when calling str->data() or str->c_str() is also incorrect. - // Some implementations of basic_string use a copy-on-write approach and - // this could end up mutating the data that is shared across multiple string - // objects. - if (length_with_null <= 1) - return NULL; - return &((*str)[0]); } diff --git a/base/string_util_unittest.cc b/base/string_util_unittest.cc index 1ffc59b..a028882 100644 --- a/base/string_util_unittest.cc +++ b/base/string_util_unittest.cc @@ -1113,34 +1113,27 @@ TEST(StringUtilTest, ContainsOnlyChars) { EXPECT_FALSE(ContainsOnlyChars("123a", "4321")); } -TEST(StringUtilTest, WriteInto) { +class WriteIntoTest : public testing::Test { + protected: + static void WritesCorrectly(size_t num_chars) { + std::string buffer; + char kOriginal[] = "supercali"; + strncpy(WriteInto(&buffer, num_chars + 1), kOriginal, num_chars); + // Using std::string(buffer.c_str()) instead of |buffer| truncates the + // string at the first \0. + EXPECT_EQ(std::string(kOriginal, + std::min(num_chars, arraysize(kOriginal) - 1)), + std::string(buffer.c_str())); + EXPECT_EQ(num_chars, buffer.size()); + } +}; + +TEST_F(WriteIntoTest, WriteInto) { // Validate that WriteInto reserves enough space and // sizes a string correctly. - std::string buffer; - const char kOriginal[] = "supercali"; - strncpy(WriteInto(&buffer, 1), kOriginal, 0); - EXPECT_STREQ("", buffer.c_str()); - EXPECT_EQ(0u, buffer.size()); - strncpy(WriteInto(&buffer, 2), kOriginal, 1); - EXPECT_STREQ("s", buffer.c_str()); - EXPECT_EQ(1u, buffer.size()); - strncpy(WriteInto(&buffer, 3), kOriginal, 2); - EXPECT_STREQ("su", buffer.c_str()); - EXPECT_EQ(2u, buffer.size()); - strncpy(WriteInto(&buffer, 5001), kOriginal, 5000); - EXPECT_STREQ("supercali", buffer.c_str()); - EXPECT_EQ(5000u, buffer.size()); - strncpy(WriteInto(&buffer, 3), kOriginal, 2); - EXPECT_STREQ("su", buffer.c_str()); - EXPECT_EQ(2u, buffer.size()); - strncpy(WriteInto(&buffer, 1), kOriginal, 0); - EXPECT_STREQ("", buffer.c_str()); - EXPECT_EQ(0u, buffer.size()); - - // Validate that WriteInto returns NULL only when - // |length_with_null| == 1. - EXPECT_TRUE(WriteInto(&buffer, 1) == NULL); - EXPECT_TRUE(WriteInto(&buffer, 2) != NULL); + WritesCorrectly(1); + WritesCorrectly(2); + WritesCorrectly(5000); // Validate that WriteInto doesn't modify other strings // when using a Copy-on-Write implementation. @@ -1149,9 +1142,9 @@ TEST(StringUtilTest, WriteInto) { const std::string live = kLive; std::string dead = live; strncpy(WriteInto(&dead, 5), kDead, 4); - EXPECT_STREQ(kDead, dead.c_str()); + EXPECT_EQ(kDead, dead); EXPECT_EQ(4u, dead.size()); - EXPECT_STREQ(kLive, live.c_str()); + EXPECT_EQ(kLive, live); EXPECT_EQ(4u, live.size()); } diff --git a/chrome/browser/automation/testing_automation_provider_win.cc b/chrome/browser/automation/testing_automation_provider_win.cc index 424aa39..21d2137 100644 --- a/chrome/browser/automation/testing_automation_provider_win.cc +++ b/chrome/browser/automation/testing_automation_provider_win.cc @@ -86,9 +86,10 @@ void TestingAutomationProvider::SetWindowVisible(int handle, void TestingAutomationProvider::GetWindowTitle(int handle, string16* text) { gfx::NativeWindow window = window_tracker_->GetResource(handle); - std::wstring result; int length = ::GetWindowTextLength(window) + 1; - ::GetWindowText(window, WriteInto(&result, length), length); - text->assign(WideToUTF16(result)); + if (length > 1) + ::GetWindowText(window, WriteInto(text, length), length); + else + text->clear(); } diff --git a/chrome/browser/browser_focus_uitest.cc b/chrome/browser/browser_focus_uitest.cc index 0f4fdcc..dada7af 100644 --- a/chrome/browser/browser_focus_uitest.cc +++ b/chrome/browser/browser_focus_uitest.cc @@ -90,7 +90,8 @@ bool ChromeInForeground() { std::wstring caption; std::wstring filename; int len = ::GetWindowTextLength(window) + 1; - ::GetWindowText(window, WriteInto(&caption, len), len); + if (len > 1) + ::GetWindowText(window, WriteInto(&caption, len), len); bool chrome_window_in_foreground = EndsWith(caption, L" - Google Chrome", true) || EndsWith(caption, L" - Chromium", true); @@ -102,8 +103,8 @@ bool ChromeInForeground() { if (base::OpenProcessHandleWithAccess(process_id, PROCESS_QUERY_LIMITED_INFORMATION, &process)) { - len = MAX_PATH; - if (!GetProcessImageFileName(process, WriteInto(&filename, len), len)) { + if (!GetProcessImageFileName(process, WriteInto(&filename, MAX_PATH), + MAX_PATH)) { int error = GetLastError(); filename = std::wstring(L"Unable to read filename for process id '" + base::IntToString16(process_id) + diff --git a/chrome/browser/mac/install_from_dmg.mm b/chrome/browser/mac/install_from_dmg.mm index b263491..6f276b8 100644 --- a/chrome/browser/mac/install_from_dmg.mm +++ b/chrome/browser/mac/install_from_dmg.mm @@ -141,6 +141,10 @@ bool MediaResidesOnDiskImage(io_service_t media, std::string* image_path) { CFDataRef image_path_data = static_cast<CFDataRef>( image_path_cftyperef.get()); CFIndex length = CFDataGetLength(image_path_data); + if (length <= 0) { + LOG(ERROR) << "image_path_data is unexpectedly empty"; + return true; + } char* image_path_c = WriteInto(image_path, length + 1); CFDataGetBytes(image_path_data, CFRangeMake(0, length), diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc index fd0ca98..379d94d 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc @@ -645,11 +645,9 @@ void OmniboxViewWin::OpenMatch(const AutocompleteMatch& match, string16 OmniboxViewWin::GetText() const { const int len = GetTextLength() + 1; - if (len <= 1) - return string16(); - string16 str; - GetWindowText(WriteInto(&str, len), len); + if (len > 1) + GetWindowText(WriteInto(&str, len), len); return str; } @@ -2128,15 +2126,11 @@ void OmniboxViewWin::GetSelection(CHARRANGE& sel) const { } string16 OmniboxViewWin::GetSelectedText() const { - // Figure out the length of the selection. CHARRANGE sel; GetSel(sel); - if (sel.cpMin == sel.cpMax) // GetSelText() crashes on NULL input. - return string16(); - - // Grab the selected text. string16 str; - GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1)); + if (sel.cpMin != sel.cpMax) + GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1)); return str; } diff --git a/chrome/common/metrics_log_manager.cc b/chrome/common/metrics_log_manager.cc index 37a9e4b..960672a 100644 --- a/chrome/common/metrics_log_manager.cc +++ b/chrome/common/metrics_log_manager.cc @@ -105,7 +105,7 @@ void MetricsLogManager::LoadPersistedUnsentLogs() { void MetricsLogManager::CompressStagedLog() { int text_size = staged_log_->GetEncodedLogSize(); std::string staged_log_text; - // Leave room for the NULL terminator. + DCHECK_GT(text_size, 0); staged_log_->GetEncodedLog(WriteInto(&staged_log_text, text_size + 1), text_size); diff --git a/chrome/common/time_format.cc b/chrome/common/time_format.cc index 75407a9..c4cf9ca 100644 --- a/chrome/common/time_format.cc +++ b/chrome/common/time_format.cc @@ -296,9 +296,9 @@ static string16 FormatTimeImpl(const TimeDelta& delta, FormatType format_type) { // With the fallback added, this should never fail. DCHECK(U_SUCCESS(error)); int capacity = time_string.length() + 1; + DCHECK_GT(capacity, 1); string16 result; - time_string.extract(static_cast<UChar*>( - WriteInto(&result, capacity)), + time_string.extract(static_cast<UChar*>(WriteInto(&result, capacity)), capacity, error); DCHECK(U_SUCCESS(error)); return result; diff --git a/chrome_frame/chrome_tab.cc b/chrome_frame/chrome_tab.cc index b394a11..b984e73 100644 --- a/chrome_frame/chrome_tab.cc +++ b/chrome_frame/chrome_tab.cc @@ -467,8 +467,9 @@ class SecurityDescBackup { DWORD reg_type = REG_NONE; if (backup_key.ReadValue(NULL, NULL, &len, ®_type) != ERROR_SUCCESS) return false; + DCHECK_EQ(0u, len % sizeof(wchar_t)); - if (reg_type != REG_SZ) + if ((len == 0) || (reg_type != REG_SZ)) return false; size_t wchar_count = 1 + len / sizeof(wchar_t); diff --git a/chrome_frame/test/win_event_receiver.cc b/chrome_frame/test/win_event_receiver.cc index 045a48c..fbc21f2 100644 --- a/chrome_frame/test/win_event_receiver.cc +++ b/chrome_frame/test/win_event_receiver.cc @@ -169,8 +169,8 @@ void WindowWatchdog::RemoveObserver(WindowObserver* observer) { std::string WindowWatchdog::GetWindowCaption(HWND hwnd) { std::string caption; int len = ::GetWindowTextLength(hwnd) + 1; - ::GetWindowTextA(hwnd, WriteInto(&caption, len), len); - + if (len > 1) + ::GetWindowTextA(hwnd, WriteInto(&caption, len), len); return caption; } diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index 7f48648..9003109 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -1328,6 +1328,7 @@ std::string Bscf2Str(DWORD flags) { // Reads data from a stream into a string. HRESULT ReadStream(IStream* stream, size_t size, std::string* data) { DCHECK(stream); + DCHECK_GT(size, 0u); DCHECK(data); DWORD read = 0; diff --git a/crypto/encryptor.h b/crypto/encryptor.h index a407266..6b23619 100644 --- a/crypto/encryptor.h +++ b/crypto/encryptor.h @@ -61,13 +61,15 @@ class CRYPTO_EXPORT Encryptor { // Initializes the encryptor using |key| and |iv|. Returns false if either the // key or the initialization vector cannot be used. // - // When |mode| is CTR then |iv| should be empty. + // If |mode| is CBC, |iv| must not be empty; if it is CTR, then |iv| must be + // empty. bool Init(SymmetricKey* key, Mode mode, const base::StringPiece& iv); - // Encrypts |plaintext| into |ciphertext|. + // Encrypts |plaintext| into |ciphertext|. |plaintext| may only be empty if + // the mode is CBC. bool Encrypt(const base::StringPiece& plaintext, std::string* ciphertext); - // Decrypts |ciphertext| into |plaintext|. + // Decrypts |ciphertext| into |plaintext|. |ciphertext| must not be empty. bool Decrypt(const base::StringPiece& ciphertext, std::string* plaintext); // Sets the counter value when in CTR mode. Currently only 128-bits diff --git a/crypto/encryptor_mac.cc b/crypto/encryptor_mac.cc index a08d09e..6be373a 100644 --- a/crypto/encryptor_mac.cc +++ b/crypto/encryptor_mac.cc @@ -49,17 +49,19 @@ bool Encryptor::Crypt(int /*CCOperation*/ op, // length is never larger than the input length plus the block size." size_t output_size = input.size() + iv_.size(); + CHECK_GT(output_size, 0u); + CHECK_GT(output_size + 1, input.size()); CCCryptorStatus err = CCCrypt(op, kCCAlgorithmAES128, kCCOptionPKCS7Padding, raw_key.Data, raw_key.Length, iv_.data(), input.data(), input.size(), - WriteInto(output, output_size+1), + WriteInto(output, output_size + 1), output_size, &output_size); if (err) { - output->resize(0); + output->clear(); LOG(ERROR) << "CCCrypt returned " << err; return false; } @@ -69,11 +71,13 @@ bool Encryptor::Crypt(int /*CCOperation*/ op, bool Encryptor::Encrypt(const base::StringPiece& plaintext, std::string* ciphertext) { + CHECK(!plaintext.empty() || (mode_ == CBC)); return Crypt(kCCEncrypt, plaintext, ciphertext); } bool Encryptor::Decrypt(const base::StringPiece& ciphertext, std::string* plaintext) { + CHECK(!ciphertext.empty()); return Crypt(kCCDecrypt, ciphertext, plaintext); } diff --git a/crypto/encryptor_nss.cc b/crypto/encryptor_nss.cc index c4610903..cf4fa2a 100644 --- a/crypto/encryptor_nss.cc +++ b/crypto/encryptor_nss.cc @@ -72,13 +72,12 @@ bool Encryptor::Init(SymmetricKey* key, break; } - if (!param_.get()) - return false; - return true; + return param_ != NULL; } bool Encryptor::Encrypt(const base::StringPiece& plaintext, std::string* ciphertext) { + CHECK(!plaintext.empty() || (mode_ == CBC)); ScopedPK11Context context(PK11_CreateContextBySymKey(GetMechanism(mode_), CKA_ENCRYPT, key_->key(), @@ -86,34 +85,30 @@ bool Encryptor::Encrypt(const base::StringPiece& plaintext, if (!context.get()) return false; - if (mode_ == CTR) - return CryptCTR(context.get(), plaintext, ciphertext); - else - return Crypt(context.get(), plaintext, ciphertext); + return (mode_ == CTR) ? + CryptCTR(context.get(), plaintext, ciphertext) : + Crypt(context.get(), plaintext, ciphertext); } bool Encryptor::Decrypt(const base::StringPiece& ciphertext, std::string* plaintext) { - if (ciphertext.empty()) - return false; - + CHECK(!ciphertext.empty()); ScopedPK11Context context(PK11_CreateContextBySymKey( GetMechanism(mode_), (mode_ == CTR ? CKA_ENCRYPT : CKA_DECRYPT), key_->key(), param_.get())); if (!context.get()) return false; - if (mode_ == CTR) - return CryptCTR(context.get(), ciphertext, plaintext); - else - return Crypt(context.get(), ciphertext, plaintext); + return (mode_ == CTR) ? + CryptCTR(context.get(), ciphertext, plaintext) : + Crypt(context.get(), ciphertext, plaintext); } bool Encryptor::Crypt(PK11Context* context, const base::StringPiece& input, std::string* output) { size_t output_len = input.size() + AES_BLOCK_SIZE; - CHECK(output_len > input.size()) << "Output size overflow"; + CHECK_GT(output_len, input.size()); output->resize(output_len); uint8* output_data = @@ -160,7 +155,7 @@ bool Encryptor::CryptCTR(PK11Context* context, size_t output_len = ((input.size() + AES_BLOCK_SIZE - 1) / AES_BLOCK_SIZE) * AES_BLOCK_SIZE; - CHECK(output_len >= input.size()) << "Output size overflow"; + CHECK_GE(output_len, input.size()); output->resize(output_len); uint8* output_data = reinterpret_cast<uint8*>(const_cast<char*>(output->data())); @@ -180,7 +175,7 @@ bool Encryptor::CryptCTR(PK11Context* context, mask_len); if (SECSuccess != rv) return false; - CHECK(op_len == static_cast<int>(mask_len)); + CHECK_EQ(static_cast<int>(mask_len), op_len); unsigned int digest_len; rv = PK11_DigestFinal(context, diff --git a/crypto/encryptor_openssl.cc b/crypto/encryptor_openssl.cc index 6513181..cb26e22 100644 --- a/crypto/encryptor_openssl.cc +++ b/crypto/encryptor_openssl.cc @@ -73,11 +73,13 @@ bool Encryptor::Init(SymmetricKey* key, bool Encryptor::Encrypt(const base::StringPiece& plaintext, std::string* ciphertext) { + CHECK(!plaintext.empty() || (mode_ == CBC)); return Crypt(true, plaintext, ciphertext); } bool Encryptor::Decrypt(const base::StringPiece& ciphertext, std::string* plaintext) { + CHECK(!ciphertext.empty()); return Crypt(false, ciphertext, plaintext); } @@ -88,7 +90,7 @@ bool Encryptor::Crypt(bool do_encrypt, // Work on the result in a local variable, and then only transfer it to // |output| on success to ensure no partial data is returned. std::string result; - output->swap(result); + output->clear(); const EVP_CIPHER* cipher = GetCipherForKey(key_); DCHECK(cipher); // Already handled in Init(); @@ -106,6 +108,8 @@ bool Encryptor::Crypt(bool do_encrypt, // When encrypting, add another block size of space to allow for any padding. const size_t output_size = input.size() + (do_encrypt ? iv_.size() : 0); + CHECK_GT(output_size, 0u); + CHECK_GT(output_size + 1, input.size()); uint8* out_ptr = reinterpret_cast<uint8*>(WriteInto(&result, output_size + 1)); int out_len; diff --git a/crypto/encryptor_unittest.cc b/crypto/encryptor_unittest.cc index 2d710bc..7f2caf9 100644 --- a/crypto/encryptor_unittest.cc +++ b/crypto/encryptor_unittest.cc @@ -281,21 +281,3 @@ TEST(EncryptorTest, EmptyEncrypt) { EXPECT_EQ(expected_ciphertext_hex, base::HexEncode(ciphertext.data(), ciphertext.size())); } - -TEST(EncryptorTest, EmptyDecrypt) { - std::string key = "128=SixteenBytes"; - std::string iv = "Sweet Sixteen IV"; - - scoped_ptr<crypto::SymmetricKey> sym_key(crypto::SymmetricKey::Import( - crypto::SymmetricKey::AES, key)); - ASSERT_TRUE(NULL != sym_key.get()); - - crypto::Encryptor encryptor; - // The IV must be exactly as long a the cipher block size. - EXPECT_EQ(16U, iv.size()); - EXPECT_TRUE(encryptor.Init(sym_key.get(), crypto::Encryptor::CBC, iv)); - - std::string decrypted; - EXPECT_FALSE(encryptor.Decrypt("", &decrypted)); - EXPECT_EQ("", decrypted); -} diff --git a/crypto/encryptor_win.cc b/crypto/encryptor_win.cc index fae1f82..dc59519 100644 --- a/crypto/encryptor_win.cc +++ b/crypto/encryptor_win.cc @@ -83,8 +83,10 @@ bool Encryptor::Init(SymmetricKey* key, bool Encryptor::Encrypt(const base::StringPiece& plaintext, std::string* ciphertext) { DWORD data_len = plaintext.size(); + CHECK((data_len > 0u) || (mode_ == CBC)); DWORD total_len = data_len + block_size_; - CHECK_GT(total_len, data_len); + CHECK_GT(total_len, 0u); + CHECK_GT(total_len + 1, data_len); // CryptoAPI encrypts/decrypts in place. char* ciphertext_data = WriteInto(ciphertext, total_len + 1); @@ -105,8 +107,8 @@ bool Encryptor::Encrypt(const base::StringPiece& plaintext, bool Encryptor::Decrypt(const base::StringPiece& ciphertext, std::string* plaintext) { DWORD data_len = ciphertext.size(); - if (data_len == 0 || (data_len + 1) < data_len) - return false; + CHECK_GT(data_len, 0u); + CHECK_GT(data_len + 1, data_len); // CryptoAPI encrypts/decrypts in place. char* plaintext_data = WriteInto(plaintext, data_len + 1); diff --git a/crypto/symmetric_key.h b/crypto/symmetric_key.h index 500324e..3c25fab 100644 --- a/crypto/symmetric_key.h +++ b/crypto/symmetric_key.h @@ -35,15 +35,16 @@ class CRYPTO_EXPORT SymmetricKey { virtual ~SymmetricKey(); // Generates a random key suitable to be used with |algorithm| and of - // |key_size_in_bits| bits. + // |key_size_in_bits| bits. |key_size_in_bits| must be a multiple of 8. // The caller is responsible for deleting the returned SymmetricKey. static SymmetricKey* GenerateRandomKey(Algorithm algorithm, size_t key_size_in_bits); // Derives a key from the supplied password and salt using PBKDF2, suitable // for use with specified |algorithm|. Note |algorithm| is not the algorithm - // used to derive the key from the password. The caller is responsible for - // deleting the returned SymmetricKey. + // used to derive the key from the password. |key_size_in_bits| must be a + // multiple of 8. The caller is responsible for deleting the returned + // SymmetricKey. static SymmetricKey* DeriveKeyFromPassword(Algorithm algorithm, const std::string& password, const std::string& salt, diff --git a/crypto/symmetric_key_openssl.cc b/crypto/symmetric_key_openssl.cc index 1d1ad23..899a942 100644 --- a/crypto/symmetric_key_openssl.cc +++ b/crypto/symmetric_key_openssl.cc @@ -24,10 +24,10 @@ SymmetricKey::~SymmetricKey() { SymmetricKey* SymmetricKey::GenerateRandomKey(Algorithm algorithm, size_t key_size_in_bits) { DCHECK_EQ(AES, algorithm); - int key_size_in_bytes = key_size_in_bits / 8; - DCHECK_EQ(static_cast<int>(key_size_in_bits), key_size_in_bytes * 8); + size_t key_size_in_bytes = key_size_in_bits / 8; + DCHECK_EQ(key_size_in_bits, key_size_in_bytes * 8); - if (key_size_in_bits == 0) + if (key_size_in_bytes == 0) return NULL; OpenSSLErrStackTracer err_tracer(FROM_HERE); @@ -35,7 +35,7 @@ SymmetricKey* SymmetricKey::GenerateRandomKey(Algorithm algorithm, uint8* key_data = reinterpret_cast<uint8*>(WriteInto(&key->key_, key_size_in_bytes + 1)); - int rv = RAND_bytes(key_data, key_size_in_bytes); + int rv = RAND_bytes(key_data, static_cast<int>(key_size_in_bytes)); return rv == 1 ? key.release() : NULL; } @@ -46,8 +46,11 @@ SymmetricKey* SymmetricKey::DeriveKeyFromPassword(Algorithm algorithm, size_t iterations, size_t key_size_in_bits) { DCHECK(algorithm == AES || algorithm == HMAC_SHA1); - int key_size_in_bytes = key_size_in_bits / 8; - DCHECK_EQ(static_cast<int>(key_size_in_bits), key_size_in_bytes * 8); + size_t key_size_in_bytes = key_size_in_bits / 8; + DCHECK_EQ(key_size_in_bits, key_size_in_bytes * 8); + + if (key_size_in_bytes == 0) + return NULL; OpenSSLErrStackTracer err_tracer(FROM_HERE); scoped_ptr<SymmetricKey> key(new SymmetricKey); @@ -56,7 +59,8 @@ SymmetricKey* SymmetricKey::DeriveKeyFromPassword(Algorithm algorithm, int rv = PKCS5_PBKDF2_HMAC_SHA1(password.data(), password.length(), reinterpret_cast<const uint8*>(salt.data()), salt.length(), iterations, - key_size_in_bytes, key_data); + static_cast<int>(key_size_in_bytes), + key_data); return rv == 1 ? key.release() : NULL; } diff --git a/media/audio/win/audio_manager_win.cc b/media/audio/win/audio_manager_win.cc index 92f59d8..80d0d9d 100644 --- a/media/audio/win/audio_manager_win.cc +++ b/media/audio/win/audio_manager_win.cc @@ -211,12 +211,14 @@ string16 AudioManagerWin::GetAudioInputDeviceModel() { waveInMessage(reinterpret_cast<HWAVEIN>(device_id), DRV_QUERYDEVICEINTERFACESIZE, reinterpret_cast<DWORD_PTR>(&device_interface_name_size), 0); - if (device_interface_name_size == 0) // No audio capture device? - return string16(); + size_t bytes_in_char16 = sizeof(string16::value_type); + DCHECK_EQ(0u, device_interface_name_size % bytes_in_char16); + if (device_interface_name_size <= bytes_in_char16) + return string16(); // No audio capture device. string16 device_interface_name; string16::value_type* name_ptr = WriteInto(&device_interface_name, - device_interface_name_size / sizeof(string16::value_type)); + device_interface_name_size / bytes_in_char16); waveInMessage(reinterpret_cast<HWAVEIN>(device_id), DRV_QUERYDEVICEINTERFACE, reinterpret_cast<DWORD_PTR>(name_ptr), diff --git a/net/base/net_util.cc b/net/base/net_util.cc index 204e768..7ae4363d 100644 --- a/net/base/net_util.cc +++ b/net/base/net_util.cc @@ -162,13 +162,9 @@ static const int kAllowedFtpPorts[] = { std::string::size_type CountTrailingChars( const std::string input, const std::string::value_type trailing_chars[]) { - const std::string::size_type last_good_char = - input.find_last_not_of(trailing_chars); - - if (last_good_char == std::string::npos) - return input.length(); - else - return input.length() - last_good_char - 1; + const size_t last_good_char = input.find_last_not_of(trailing_chars); + return (last_good_char == std::string::npos) ? + input.length() : (input.length() - last_good_char - 1); } // Similar to Base64Decode. Decodes a Q-encoded string to a sequence @@ -176,71 +172,68 @@ std::string::size_type CountTrailingChars( bool QPDecode(const std::string& input, std::string* output) { std::string temp; temp.reserve(input.size()); - std::string::const_iterator it = input.begin(); - while (it != input.end()) { + for (std::string::const_iterator it = input.begin(); it != input.end(); + ++it) { if (*it == '_') { temp.push_back(' '); } else if (*it == '=') { - if (input.end() - it < 3) { - return false; - } - if (IsHexDigit(static_cast<unsigned char>(*(it + 1))) && - IsHexDigit(static_cast<unsigned char>(*(it + 2)))) { - unsigned char ch = HexDigitToInt(*(it + 1)) * 16 + - HexDigitToInt(*(it + 2)); - temp.push_back(static_cast<char>(ch)); - ++it; - ++it; - } else { + if ((input.end() - it < 3) || + !IsHexDigit(static_cast<unsigned char>(*(it + 1))) || + !IsHexDigit(static_cast<unsigned char>(*(it + 2)))) return false; - } + unsigned char ch = HexDigitToInt(*(it + 1)) * 16 + + HexDigitToInt(*(it + 2)); + temp.push_back(static_cast<char>(ch)); + ++it; + ++it; } else if (0x20 < *it && *it < 0x7F) { // In a Q-encoded word, only printable ASCII characters // represent themselves. Besides, space, '=', '_' and '?' are // not allowed, but they're already filtered out. - DCHECK(*it != 0x3D && *it != 0x5F && *it != 0x3F); + DCHECK_NE('=', *it); + DCHECK_NE('?', *it); + DCHECK_NE('_', *it); temp.push_back(*it); } else { return false; } - ++it; } output->swap(temp); return true; } enum RFC2047EncodingType {Q_ENCODING, B_ENCODING}; -bool DecodeBQEncoding(const std::string& part, RFC2047EncodingType enc_type, - const std::string& charset, std::string* output) { +bool DecodeBQEncoding(const std::string& part, + RFC2047EncodingType enc_type, + const std::string& charset, + std::string* output) { std::string decoded; - if (enc_type == B_ENCODING) { - if (!base::Base64Decode(part, &decoded)) { - return false; - } - } else { - if (!QPDecode(part, &decoded)) { - return false; - } + if (!((enc_type == B_ENCODING) ? + base::Base64Decode(part, &decoded) : QPDecode(part, &decoded))) + return false; + + if (decoded.empty()) { + output->clear(); + return true; } UErrorCode err = U_ZERO_ERROR; UConverter* converter(ucnv_open(charset.c_str(), &err)); - if (U_FAILURE(err)) { + if (U_FAILURE(err)) return false; - } // A single byte in a legacy encoding can be expanded to 3 bytes in UTF-8. // A 'two-byte character' in a legacy encoding can be expanded to 4 bytes - // in UTF-8. Therefore, the expansion ratio is 3 at most. - int length = static_cast<int>(decoded.length()); - char* buf = WriteInto(output, length * 3); - length = ucnv_toAlgorithmic(UCNV_UTF8, converter, buf, length * 3, - decoded.data(), length, &err); + // in UTF-8. Therefore, the expansion ratio is 3 at most. Add one for a + // trailing '\0'. + size_t output_length = decoded.length() * 3 + 1; + char* buf = WriteInto(output, output_length); + output_length = ucnv_toAlgorithmic(UCNV_UTF8, converter, buf, output_length, + decoded.data(), decoded.length(), &err); ucnv_close(converter); - if (U_FAILURE(err)) { + if (U_FAILURE(err)) return false; - } - output->resize(length); + output->resize(output_length); return true; } diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc index 7309021..12acdaf 100644 --- a/net/base/x509_certificate_win.cc +++ b/net/base/x509_certificate_win.cc @@ -557,18 +557,22 @@ void X509Certificate::Initialize() { &cert_handle_->pCertInfo->Subject, CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG, NULL, 0); - name_size = CertNameToStr(cert_handle_->dwCertEncodingType, - &cert_handle_->pCertInfo->Subject, - CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG, - WriteInto(&subject_info, name_size), name_size); + if (name_size > 1) { + CertNameToStr(cert_handle_->dwCertEncodingType, + &cert_handle_->pCertInfo->Subject, + CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG, + WriteInto(&subject_info, name_size), name_size); + } name_size = CertNameToStr(cert_handle_->dwCertEncodingType, &cert_handle_->pCertInfo->Issuer, CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG, NULL, 0); - name_size = CertNameToStr(cert_handle_->dwCertEncodingType, - &cert_handle_->pCertInfo->Issuer, - CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG, - WriteInto(&issuer_info, name_size), name_size); + if (name_size > 1) { + CertNameToStr(cert_handle_->dwCertEncodingType, + &cert_handle_->pCertInfo->Issuer, + CERT_X500_NAME_STR | CERT_NAME_STR_CRLF_FLAG, + WriteInto(&issuer_info, name_size), name_size); + } ParsePrincipal(WideToUTF8(subject_info), &subject_); ParsePrincipal(WideToUTF8(issuer_info), &issuer_); diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc index 976e16f..0a6a2d2 100644 --- a/net/disk_cache/entry_impl.cc +++ b/net/disk_cache/entry_impl.cc @@ -773,11 +773,11 @@ std::string EntryImpl::GetKey() const { File* key_file = const_cast<EntryImpl*>(this)->GetBackingFile(address, kKeyFileIndex); - if (!offset && key_file->GetLength() != static_cast<size_t>(key_len + 1)) + ++key_len; // We store a trailing \0 on disk that we read back below. + if (!offset && key_file->GetLength() != static_cast<size_t>(key_len)) return std::string(); - if (!key_file || - !key_file->Read(WriteInto(&key_, key_len + 1), key_len + 1, offset)) + if (!key_file || !key_file->Read(WriteInto(&key_, key_len), key_len, offset)) key_.clear(); return key_; } diff --git a/net/http/http_mac_signature.cc b/net/http/http_mac_signature.cc index 93498ef..e445165 100644 --- a/net/http/http_mac_signature.cc +++ b/net/http/http_mac_signature.cc @@ -158,9 +158,12 @@ bool HttpMacSignature::GenerateMAC(const std::string& age, std::string signature; size_t length = hmac.DigestLength(); - char* buffer = WriteInto(&signature, length); - if (!hmac.Sign(request, reinterpret_cast<unsigned char*>(buffer), - length)) { + DCHECK_GT(length, 0u); + if (!hmac.Sign(request, + // We need the + 1 here not because the call will write a trailing \0, + // but so that signature.length() is correctly set to |length|. + reinterpret_cast<unsigned char*>(WriteInto(&signature, length + 1)), + length)) { NOTREACHED(); return false; } diff --git a/net/http/http_mac_signature_unittest.cc b/net/http/http_mac_signature_unittest.cc index 74064dd..130ce39 100644 --- a/net/http/http_mac_signature_unittest.cc +++ b/net/http/http_mac_signature_unittest.cc @@ -59,7 +59,7 @@ TEST(HttpMacSignatureTest, GenerateHeaderString) { EXPECT_TRUE(signature.GenerateHeaderString(age, nonce, &header_string)); EXPECT_EQ("MAC id=\"dfoi30j0qnf\", " "nonce=\"239034:mn4302j0n+32r2/f3r=\", " - "mac=\"GrkHtPKzB1m1dCHfa7OCWOw6EQ==\"", + "mac=\"GrkHtPKzB1m1dCHfa7OCWOw6Ecw=\"", header_string); } @@ -104,6 +104,6 @@ TEST(HttpMacSignatureTest, GenerateMAC) { std::string mac; EXPECT_TRUE(signature.GenerateMAC(age, nonce, &mac)); - EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6EQ==", mac); + EXPECT_EQ("GrkHtPKzB1m1dCHfa7OCWOw6Ecw=", mac); } } diff --git a/sandbox/src/process_policy_test.cc b/sandbox/src/process_policy_test.cc index 8681671..1bcfa88 100644 --- a/sandbox/src/process_policy_test.cc +++ b/sandbox/src/process_policy_test.cc @@ -5,11 +5,11 @@ #include <memory> #include <string> +#include "base/sys_string_conversions.h" #include "base/win/scoped_handle.h" #include "sandbox/src/sandbox.h" #include "sandbox/src/sandbox_policy.h" #include "sandbox/src/sandbox_factory.h" -#include "sandbox/src/sandbox_utils.h" #include "sandbox/tests/common/controller.h" #include "testing/gtest/include/gtest/gtest.h" @@ -67,9 +67,9 @@ sandbox::SboxTestResult CreateProcessHelper(const std::wstring &exe, std::string narrow_cmd_line; if (cmd_line) - narrow_cmd_line = sandbox::WideToMultiByte(cmd_line); + narrow_cmd_line = base::SysWideToMultiByte(cmd_line, CP_UTF8); if (!::CreateProcessA( - exe_name ? sandbox::WideToMultiByte(exe_name).c_str() : NULL, + exe_name ? base::SysWideToMultiByte(exe_name, CP_UTF8).c_str() : NULL, cmd_line ? const_cast<char*>(narrow_cmd_line.c_str()) : NULL, NULL, NULL, FALSE, 0, NULL, NULL, &sia, &pi)) { DWORD last_error = GetLastError(); diff --git a/sandbox/src/sandbox_utils.cc b/sandbox/src/sandbox_utils.cc index b93434c..3bfa696 100644 --- a/sandbox/src/sandbox_utils.cc +++ b/sandbox/src/sandbox_utils.cc @@ -76,21 +76,4 @@ void InitObjectAttribs(const std::wstring& name, ULONG attributes, HANDLE root, InitializeObjectAttributes(obj_attr, uni_name, attributes, root, NULL); } -std::string WideToMultiByte(const std::wstring& wide) { - if (wide.length() == 0) - return std::string(); - - // compute the length of the buffer we'll need - int charcount = WideCharToMultiByte(CP_UTF8, 0, wide.c_str(), -1, - NULL, 0, NULL, NULL); - if (charcount == 0) - return std::string(); - - std::string mb; - WideCharToMultiByte(CP_UTF8, 0, wide.c_str(), -1, - WriteInto(&mb, charcount), charcount, NULL, NULL); - - return mb; -} - }; // namespace sandbox diff --git a/sandbox/src/sandbox_utils.h b/sandbox/src/sandbox_utils.h index e6cac5d..314330f 100644 --- a/sandbox/src/sandbox_utils.h +++ b/sandbox/src/sandbox_utils.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -33,18 +33,6 @@ bool IsXPSP2OrLater(); void InitObjectAttribs(const std::wstring& name, ULONG attributes, HANDLE root, OBJECT_ATTRIBUTES* obj_attr, UNICODE_STRING* uni_name); -// The next 2 functions are copied from base\string_util.h and have been -// slighty modified because we don't want to depend on ICU. -template <class string_type> -inline typename string_type::value_type* WriteInto(string_type* str, - size_t length_with_null) { - str->reserve(length_with_null); - str->resize(length_with_null - 1); - return &((*str)[0]); -} - -std::string WideToMultiByte(const std::wstring& wide); - }; // namespace sandbox #endif // SANDBOX_SRC_SANDBOX_UTILS_H__ diff --git a/sandbox/tests/common/controller.cc b/sandbox/tests/common/controller.cc index b618069..a221ee1 100644 --- a/sandbox/tests/common/controller.cc +++ b/sandbox/tests/common/controller.cc @@ -6,6 +6,7 @@ #include <string> +#include "base/sys_string_conversions.h" #include "base/win/windows_version.h" #include "sandbox/src/sandbox_factory.h" #include "sandbox/src/sandbox_utils.h" @@ -261,7 +262,7 @@ int DispatchCall(int argc, wchar_t **argv) { &module)) return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; - std::string command_name = WideToMultiByte(argv[3]); + std::string command_name = base::SysWideToMultiByte(argv[3], CP_UTF8); CommandFunction command = reinterpret_cast<CommandFunction>( ::GetProcAddress(module, command_name.c_str())); if (!command) diff --git a/ui/base/clipboard/clipboard_win.cc b/ui/base/clipboard/clipboard_win.cc index 492dd31..a543fe6 100644 --- a/ui/base/clipboard/clipboard_win.cc +++ b/ui/base/clipboard/clipboard_win.cc @@ -604,7 +604,8 @@ void Clipboard::ReadFiles(std::vector<FilePath>* files) const { if (count) { for (int i = 0; i < count; ++i) { - int size = ::DragQueryFile(drop, i, NULL, 0) + 1; + UINT size = ::DragQueryFile(drop, i, NULL, 0) + 1; + DCHECK_GT(size, 1u); std::wstring file; ::DragQueryFile(drop, i, WriteInto(&file, size), size); files->push_back(FilePath(file)); diff --git a/ui/base/l10n/l10n_util.cc b/ui/base/l10n/l10n_util.cc index 1c2aca0..28dbc43 100644 --- a/ui/base/l10n/l10n_util.cc +++ b/ui/base/l10n/l10n_util.cc @@ -478,12 +478,12 @@ string16 GetDisplayNameForLocale(const std::string& locale, locale_code = "zh-Hant"; UErrorCode error = U_ZERO_ERROR; - const int buffer_size = 1024; + const int kBufferSize = 1024; string16 display_name; int actual_size = uloc_getDisplayName(locale_code.c_str(), display_locale.c_str(), - WriteInto(&display_name, buffer_size + 1), buffer_size, &error); + WriteInto(&display_name, kBufferSize), kBufferSize - 1, &error); DCHECK(U_SUCCESS(error)); display_name.resize(actual_size); // Add an RTL mark so parentheses are properly placed. diff --git a/ui/base/win/ime_input.cc b/ui/base/win/ime_input.cc index 50bf3ea..049ab02 100644 --- a/ui/base/win/ime_input.cc +++ b/ui/base/win/ime_input.cc @@ -339,22 +339,19 @@ void ImeInput::GetCompositionInfo(HIMC imm_context, LPARAM lparam, } } -bool ImeInput::GetString(HIMC imm_context, WPARAM lparam, int type, +bool ImeInput::GetString(HIMC imm_context, + WPARAM lparam, + int type, string16* result) { - bool ret = false; - if (lparam & type) { - int string_size = ::ImmGetCompositionString(imm_context, type, NULL, 0); - if (string_size > 0) { - int string_length = string_size / sizeof(wchar_t); - wchar_t *string_data = WriteInto(result, string_length + 1); - if (string_data) { - // Fill the given result object. - ::ImmGetCompositionString(imm_context, type, string_data, string_size); - ret = true; - } - } - } - return ret; + if (!(lparam & type)) + return false; + LONG string_size = ::ImmGetCompositionString(imm_context, type, NULL, 0); + if (string_size <= 0) + return false; + DCHECK_EQ(0u, string_size % sizeof(wchar_t)); + ::ImmGetCompositionString(imm_context, type, + WriteInto(result, (string_size / sizeof(wchar_t)) + 1), string_size); + return true; } bool ImeInput::GetResult(HWND window_handle, LPARAM lparam, string16* result) { diff --git a/ui/views/controls/textfield/native_textfield_win.cc b/ui/views/controls/textfield/native_textfield_win.cc index 20cae12..3b598e1 100644 --- a/ui/views/controls/textfield/native_textfield_win.cc +++ b/ui/views/controls/textfield/native_textfield_win.cc @@ -173,18 +173,16 @@ void NativeTextfieldWin::AttachHack() { string16 NativeTextfieldWin::GetText() const { int len = GetTextLength() + 1; - if (len <= 1) - return string16(); - string16 str; - GetWindowText(WriteInto(&str, len), len); + if (len > 1) + GetWindowText(WriteInto(&str, len), len); // The text get from GetWindowText() might be wrapped with explicit bidi // control characters. Refer to UpdateText() for detail. Without such // wrapping, in RTL chrome, a pure LTR string ending with parenthesis will // not be displayed correctly in a textfield. For example, "Yahoo!" will be // displayed as "!Yahoo", and "Google (by default)" will be displayed as // "(Google (by default". - return base::i18n::StripWrappingBidiControlCharacters(WideToUTF16(str)); + return base::i18n::StripWrappingBidiControlCharacters(str); } void NativeTextfieldWin::UpdateText() { @@ -206,15 +204,11 @@ void NativeTextfieldWin::AppendText(const string16& text) { } string16 NativeTextfieldWin::GetSelectedText() const { - // Figure out the length of the selection. CHARRANGE sel; GetSel(sel); - if (sel.cpMin == sel.cpMax) // GetSelText() crashes on NULL input. - return string16(); - - // Grab the selected text. string16 str; - GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1)); + if (sel.cpMin != sel.cpMax) + GetSelText(WriteInto(&str, sel.cpMax - sel.cpMin + 1)); return str; } |