Linux: make the KWallet code a little more robust to weird pickle corruption.

BUG=70541
Review URL: http://codereview.chromium.org/6880149

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@82747 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 32 insertions, 25 deletions

diff --git a/chrome/browser/password_manager/native_backend_kwallet_x.cc b/chrome/browser/password_manager/native_backend_kwallet_x.cc
index 3ec2ec2..7305747 100644
--- a/chrome/browser/password_manager/native_backend_kwallet_x.cc
+++ b/chrome/browser/password_manager/native_backend_kwallet_x.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
 
@@ -413,7 +413,7 @@ bool NativeBackendKWallet::SetLoginsList(const PasswordFormList& forms,
                       G_TYPE_INVALID);
     CheckError();
     if (ret != 0)
-      LOG(ERROR) << "Bad return code " << ret << " from kwallet removeEntry";
+      LOG(ERROR) << "Bad return code " << ret << " from KWallet removeEntry";
     return ret == 0;
   }
 
@@ -440,7 +440,7 @@ bool NativeBackendKWallet::SetLoginsList(const PasswordFormList& forms,
 
   CheckError();
   if (ret != 0)
-    LOG(ERROR) << "Bad return code " << ret << " from kwallet writeEntry";
+    LOG(ERROR) << "Bad return code " << ret << " from KWallet writeEntry";
   return ret == 0;
 }
 
@@ -485,7 +485,7 @@ bool NativeBackendKWallet::CheckSerializedValue(const GArray* byte_array,
       reinterpret_cast<const Pickle::Header*>(byte_array->data);
   if (byte_array->len < sizeof(*header) ||
       header->payload_size > byte_array->len - sizeof(*header)) {
-    LOG(WARNING) << "Invalid KWallet entry detected! (realm: " << realm << ")";
+    LOG(WARNING) << "Invalid KWallet entry detected (realm: " << realm << ")";
     return false;
   }
   return true;
@@ -508,38 +508,45 @@ void NativeBackendKWallet::DeserializeValue(const string& signon_realm,
 
   forms->reserve(forms->size() + count);
   for (size_t i = 0; i < count; ++i) {
-    PasswordForm* form = new PasswordForm();
+    scoped_ptr<PasswordForm> form(new PasswordForm());
     form->signon_realm.assign(signon_realm);
 
     int scheme = 0;
-    pickle.ReadInt(&iter, &scheme);
-    form->scheme = static_cast<PasswordForm::Scheme>(scheme);
-    ReadGURL(pickle, &iter, &form->origin);
-    ReadGURL(pickle, &iter, &form->action);
-    pickle.ReadString16(&iter, &form->username_element);
-    pickle.ReadString16(&iter, &form->username_value);
-    pickle.ReadString16(&iter, &form->password_element);
-    pickle.ReadString16(&iter, &form->password_value);
-    pickle.ReadString16(&iter, &form->submit_element);
-    pickle.ReadBool(&iter, &form->ssl_valid);
-    pickle.ReadBool(&iter, &form->preferred);
-    pickle.ReadBool(&iter, &form->blacklisted_by_user);
     int64 date_created = 0;
-    pickle.ReadInt64(&iter, &date_created);
+    // Note that these will be read back in the order listed due to
+    // short-circuit evaluation. This is important.
+    if (!pickle.ReadInt(&iter, &scheme) ||
+        !ReadGURL(pickle, &iter, &form->origin) ||
+        !ReadGURL(pickle, &iter, &form->action) ||
+        !pickle.ReadString16(&iter, &form->username_element) ||
+        !pickle.ReadString16(&iter, &form->username_value) ||
+        !pickle.ReadString16(&iter, &form->password_element) ||
+        !pickle.ReadString16(&iter, &form->password_value) ||
+        !pickle.ReadString16(&iter, &form->submit_element) ||
+        !pickle.ReadBool(&iter, &form->ssl_valid) ||
+        !pickle.ReadBool(&iter, &form->preferred) ||
+        !pickle.ReadBool(&iter, &form->blacklisted_by_user) ||
+        !pickle.ReadInt64(&iter, &date_created)) {
+      LOG(ERROR) << "Failed to deserialize KWallet entry "
+                 << "(realm: " << signon_realm << ")";
+      break;
+    }
+    form->scheme = static_cast<PasswordForm::Scheme>(scheme);
     form->date_created = base::Time::FromTimeT(date_created);
-    forms->push_back(form);
+    forms->push_back(form.release());
   }
 }
 
-void NativeBackendKWallet::ReadGURL(const Pickle& pickle, void** iter,
+bool NativeBackendKWallet::ReadGURL(const Pickle& pickle, void** iter,
                                     GURL* url) {
   string url_string;
   if (!pickle.ReadString(iter, &url_string)) {
-    LOG(ERROR) << "Failed to read url string";
+    LOG(ERROR) << "Failed to deserialize URL";
     *url = GURL();
-    return;
+    return false;
   }
   *url = GURL(url_string);
+  return true;
 }
 
 bool NativeBackendKWallet::CheckError() {
diff --git a/chrome/browser/password_manager/native_backend_kwallet_x.h b/chrome/browser/password_manager/native_backend_kwallet_x.h
index 16e3c46..b12d8ed 100644
--- a/chrome/browser/password_manager/native_backend_kwallet_x.h
+++ b/chrome/browser/password_manager/native_backend_kwallet_x.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
 
@@ -102,8 +102,8 @@ class NativeBackendKWallet : public PasswordStoreX::NativeBackend {
                                PasswordFormList* forms);
 
   // Convenience function to read a GURL from a Pickle. Assumes the URL has
-  // been written as a std::string.
-  static void ReadGURL(const Pickle& pickle, void** iter, GURL* url);
+  // been written as a std::string. Returns true on success.
+  static bool ReadGURL(const Pickle& pickle, void** iter, GURL* url);
 
   // In case the fields in the pickle ever change, version them so we can try to
   // read old pickles. (Note: do not eat old pickles past the expiration date.)