Unify the error checking of crypto::Encryptor and add WARN_UNUSED_RESULT to prevent misuse.

crypto/encryptor_nss.cc

 // 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.
 
 #include "crypto/encryptor.h"
 
 #include <cryptohi.h>
 #include <vector>
 
 #include "base/logging.h"
+#include "base/string_util.h"
 #include "crypto/nss_util.h"
 #include "crypto/symmetric_key.h"
 
 namespace crypto {
 
 namespace {
 
 inline CK_MECHANISM_TYPE GetMechanism(Encryptor::Mode mode) {
   switch (mode) {
     case Encryptor::CBC:
@@ skipping 16 matching lines @@
       mode_(CBC) {
   EnsureNSSInit();
 }
 
 Encryptor::~Encryptor() {
 }
 
 bool Encryptor::Init(SymmetricKey* key,
                      Mode mode,
                      const base::StringPiece& iv) {
-  DCHECK(key);
-  DCHECK(CBC == mode || CTR == mode) << "Unsupported mode of operation";
+  DCHECK(CBC == mode || CTR == mode);
+  if (!key)
+    return false;
 
   key_ = key;
   mode_ = mode;
 
   if (mode == CBC && iv.size() != AES_BLOCK_SIZE)
     return false;
 
   slot_.reset(PK11_GetBestSlot(GetMechanism(mode), NULL));
   if (!slot_.get())
     return false;
@@ skipping 11 matching lines @@
     case CTR:
       param_.reset(PK11_ParamFromIV(GetMechanism(mode), NULL));
       break;
   }
 
   return param_ != NULL;
 }
 
 bool Encryptor::Encrypt(const base::StringPiece& plaintext,
                         std::string* ciphertext) {
-  CHECK(!plaintext.empty() || (mode_ == CBC));
+  if (plaintext.empty() && mode_ != CBC) {
+    ciphertext->clear();

[wtc, 2011/12/15 02:03:58]

The clear() calls before returning false can all be omitted.

+    return false;
+  }
+
   ScopedPK11Context context(PK11_CreateContextBySymKey(GetMechanism(mode_),
                                                        CKA_ENCRYPT,
                                                        key_->key(),
                                                        param_.get()));
-  if (!context.get())
+  if (!context.get()) {
+    ciphertext->clear();
     return false;
+  }
 
   return (mode_ == CTR) ?
       CryptCTR(context.get(), plaintext, ciphertext) :
       Crypt(context.get(), plaintext, ciphertext);
 }
 
 bool Encryptor::Decrypt(const base::StringPiece& ciphertext,
                         std::string* plaintext) {
-  CHECK(!ciphertext.empty());
+  if (ciphertext.empty()) {
+    plaintext->clear();
+    return false;
+  }
+
   ScopedPK11Context context(PK11_CreateContextBySymKey(
       GetMechanism(mode_), (mode_ == CTR ? CKA_ENCRYPT : CKA_DECRYPT),
       key_->key(), param_.get()));
-  if (!context.get())
+  if (!context.get()) {
+    plaintext->clear();
     return false;
+  }
 
   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) {
+  std::string result;
+  output->swap(result);
+
   size_t output_len = input.size() + AES_BLOCK_SIZE;
-  CHECK_GT(output_len, input.size());
-
-  output->resize(output_len);
-  uint8* output_data =
-      reinterpret_cast<uint8*>(const_cast<char*>(output->data()));
+  if (output_len == 0 || output_len < input.size() ||
+      output_len + 1 < input.size()) {
+    return false;  // size_t overflow.
+  }
 
   int input_len = input.size();
   uint8* input_data =
       reinterpret_cast<uint8*>(const_cast<char*>(input.data()));
+  uint8* result_data =
+      reinterpret_cast<uint8*>(WriteInto(&result, output_len + 1));
 
   int op_len;
-  SECStatus rv = PK11_CipherOp(context,
-                               output_data,
-                               &op_len,
-                               output_len,
-                               input_data,
-                               input_len);
-
-  if (SECSuccess != rv) {
-    output->clear();
+  SECStatus rv = PK11_CipherOp(context, result_data, &op_len, output_len,
+                               input_data, input_len);
+  if (SECSuccess != rv)
     return false;
-  }
 
   unsigned int digest_len;
-  rv = PK11_DigestFinal(context,
-                        output_data + op_len,
-                        &digest_len,
+  rv = PK11_DigestFinal(context, result_data + op_len, &digest_len,
                         output_len - op_len);
-  if (SECSuccess != rv) {
-    output->clear();
+  if (SECSuccess != rv)
     return false;
-  }
 
-  output->resize(op_len + digest_len);
+  result.resize(op_len + digest_len);
+  output->swap(result);
   return true;
 }
 
 bool Encryptor::CryptCTR(PK11Context* context,
                          const base::StringPiece& input,
                          std::string* output) {
+  // 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);
+
   if (!counter_.get()) {
     LOG(ERROR) << "Counter value not set in CTR mode.";
     return false;
   }
 
   size_t output_len = ((input.size() + AES_BLOCK_SIZE - 1) / AES_BLOCK_SIZE) *
       AES_BLOCK_SIZE;
-  CHECK_GE(output_len, input.size());
-  output->resize(output_len);
-  uint8* output_data =
-      reinterpret_cast<uint8*>(const_cast<char*>(output->data()));
+  if (output_len == 0 || output_len < input.size() ||
+      output_len + 1 < input.size()) {
+    return false;  // size_t overflow.
+  }
+
+  uint8* result_data =
+      reinterpret_cast<uint8*>(WriteInto(&result, output_len + 1));
 
   size_t mask_len;
-  bool ret = GenerateCounterMask(input.size(), output_data, &mask_len);
+  bool ret = GenerateCounterMask(input.size(), result_data, &mask_len);
   if (!ret)
     return false;
 
   CHECK_EQ(mask_len, output_len);
   int op_len;
-  SECStatus rv = PK11_CipherOp(context,
-                               output_data,
-                               &op_len,
-                               output_len,
-                               output_data,
-                               mask_len);
+  SECStatus rv = PK11_CipherOp(context, result_data, &op_len, output_len,
+                               result_data, mask_len);
   if (SECSuccess != rv)
     return false;
   CHECK_EQ(static_cast<int>(mask_len), op_len);
 
   unsigned int digest_len;
-  rv = PK11_DigestFinal(context,
-                        NULL,
-                        &digest_len,
-                        0);
+  rv = PK11_DigestFinal(context, NULL, &digest_len, 0);
   if (SECSuccess != rv)
     return false;
   CHECK(!digest_len);
 
-  // Use |output_data| to mask |input|.
-  MaskMessage(
-      reinterpret_cast<uint8*>(const_cast<char*>(input.data())),
-      input.length(), output_data, output_data);
-  output->resize(input.length());
+  // Use |result_data| to mask |input|.
+  MaskMessage(reinterpret_cast<uint8*>(const_cast<char*>(input.data())),
+              input.size(), result_data, result_data);
+  result.resize(input.size());
+  output->swap(result);
   return true;
 }
 
 }  // namespace crypto