patch for reading past end of input string

[Dominic Jonak]

FYI, I ran into an issue with keyczar reading uninitialized memory.  Turns out the input string is read 4 bytes at a time regardless of length, so I fixed it by adding some padding.  I'm unable to "git push" the change (presumably I don't have permission, though I'm getting a "Repository not found" error) so here's the patch:

$ git diff 5b45a8c83861da8d6c8c9f3ed72fd005f7d20c62 0dd65e736e66ec633edf7cbfae119a4ddf23294f

diff --git a/cpp/src/keyczar/base/base64w.cc b/cpp/src/keyczar/base/base64w.cc

index 5baf077..f69c078 100644

--- a/cpp/src/keyczar/base/base64w.cc

+++ b/cpp/src/keyczar/base/base64w.cc

@@ -31,8 +31,14 @@ bool Base64WDecode(const std::string& input, std::string* output) {

     return false;

 

   const std::string::size_type last_good_char = input.find_last_not_of("= ");

+  std::string substr = input.substr(0, last_good_char+1);

+  const int len = substr.size();

 

-  output->assign(modp::b64w_decode(input.substr(0, last_good_char+1)));

+  // b64w_decode() reads sizeof(uint32_t) bytes at a time and goes past the end

+  // of the string.  Pad the input to ensure the reads are valid.

+  substr.append(sizeof(uint32_t), '\0');

+

+  output->assign(modp::b64w_decode(substr.data(), len));

 

   if (output->empty())

     return false;

[Jay Tuley]

Dominic, for submitting a patch, you can make a public clone and push your changes to it.

I'm not a c++ developer, but looking at the code, I'm wonder if the right way to fix this, is to uncomment DOPAD  & define CHARPAD to = and then in  Base64WDecode add base64 padding if necessary and strip it on Base64WEncode. The comments in the code for modp seem clear that it wasn't intended to be run without DOPAD defined, and I think this reading past the end of the string demonstrates that.

-Jay