user1304765
user1304765

Reputation: 247

Crypto++ SHA1 Function

I can't figure out what's wrong with my function - it causes a breakpoint on return.

std::string generateHash(std::string source)
{
    CryptoPP::SHA1 hash;
    byte digest[CryptoPP::SHA1::DIGESTSIZE];
    hash.CalculateDigest(digest, (const byte*)source.c_str(), source.size());
    std::string output;
    CryptoPP::HexEncoder encoder;
    CryptoPP::StringSink test = CryptoPP::StringSink(output);
    encoder.Attach((CryptoPP::BufferedTransformation*)&CryptoPP::StringSink(output));
    encoder.Put(digest, sizeof(digest));
    encoder.MessageEnd();
    return output; // here
}

Upvotes: 3

Views: 1542

Answers (2)

jww
jww

Reputation: 102246

   CryptoPP::HexEncoder encoder;
   CryptoPP::StringSink test = CryptoPP::StringSink(output);
   encoder.Attach((CryptoPP::BufferedTransformation*)&CryptoPP::StringSink(output));

Onemouth's solution is correct, but here's why you are having trouble:

  • The StringSink(output) is a temporary on the stack, and it will be cleaned up when the StringSink destructors run
  • HexEncoder 'owns' the pointer to StringSink(output), so the HexEncoder will call delete on StringSink(output)

So the StringSink is cleaned up twice.

For completeness, the StringSink does not delete the string output because it takes a reference, not a pointer.

This is documented behavior in the README.txt:

88  *** Important Usage Notes ***
89  
90  1. If a constructor for A takes a pointer to an object B (except primitive
91  types such as int and char), then A owns B and will delete B at A's
92  destruction.  If a constructor for A takes a reference to an object B,
93  then the caller retains ownership of B and should not destroy it until
94  A no longer needs it. 

Though it would be highly unusual, you might be able to do what you are trying to do and fix it as follows (but I would not recommend doing it the way you are):

encoder.Attach(new Redirector(&CryptoPP::StringSink(output)));

The Redirector stops the ownership chain. When the Redirector is destroyed, it does not destroy its attached transformation (which is the StringSink). But again, its not the way to use the library.

Redirectors are useful in other places in the library, but not really in this case.

Upvotes: 2

onemouth
onemouth

Reputation: 2277

encoder.Attach((CryptoPP::BufferedTransformation*)&CryptoPP::StringSink(output));

In this line, CryptoPP::StringSink(output) is a temporary object. After this call CryptoPP::StringSink(output) no longer exists. So If you try to follow its address (encoder.Attach and encoder.Put), you will get undefined behavior. (Like you should never return the address of a local variable)

My code for reference:

 std::string generateHash(std::string source)
  {
      CryptoPP::SHA1 hash;
      byte digest[CryptoPP::SHA1::DIGESTSIZE];
      hash.CalculateDigest(digest, (const byte*)source.c_str(), source.size());
      std::string output;
      CryptoPP::HexEncoder encoder;
      CryptoPP::StringSink test = CryptoPP::StringSink(output);
      encoder.Attach(new CryptoPP::StringSink(output));
      encoder.Put(digest, sizeof(digest));
      encoder.MessageEnd();
      return output;
  }

Upvotes: 3

Related Questions