Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SHA-3 CalculateDigest results in Segmentation fault #130

Closed
GeorgeMatveev opened this issue Feb 1, 2016 · 11 comments
Closed

SHA-3 CalculateDigest results in Segmentation fault #130

GeorgeMatveev opened this issue Feb 1, 2016 · 11 comments

Comments

@GeorgeMatveev
Copy link

Hi, simple program that attempts to calculate SHA-3 hash compiles and links without any warnings but then crashes. Here's the code:

byte* digest; //resulting hash
SHA3 hash = SHA3(512);

try
{
    hash.CalculateDigest( digest, (const byte*) message.c_str(), (size_t)message.length() );
}
catch (const exception &ex)
{ cerr << "CalcDigest: " << ex.what() << endl;}

Gdg shows the following:

Program received signal SIGSEGV, Segmentation fault.
0x000000000049d61b in CryptoPP::SHA3::TruncatedFinal (this=0x7fffffffd8b0, 
    hash=0x0, size=512) at sha3.cpp:284

As it designed now method CalculateDigest returns void which makes it difficult to understand the problem. Using try catch does NOT help. It might be a good idea to use return error codes instead of void. This is already second submission of this but. First one was simply removed by someone without answering the question.

@GeorgeMatveev
Copy link
Author

More detailed output from gdb:

Program received signal SIGSEGV, Segmentation fault.
0x000000000049d61b in CryptoPP::SHA3::TruncatedFinal (this=0x7fffffffd8b0,
hash=0x0, size=512) at sha3.cpp:284
(gdb) bt
#0 0x000000000049d61b in CryptoPP::SHA3::TruncatedFinal (this=0x7fffffffd8b0,
hash=0x0, size=512) at sha3.cpp:284
#1 0x00000000004034b5 in CalculateDigest (length=,
input=, digest=, this=0x7fffffffd8b0)
at /usr/include/cryptopp/cryptlib.h:928
#2 sha3 (message=...) at sha3.cpp:32
#3 0x0000000000402fae in main (argc=, argv=)
at sha3.cpp:58
(gdb)

@GeorgeMatveev
Copy link
Author

This happens on Ubuntu/g++:

geo@geo-Lenovo-G580:/kelisec/aesha3$ g++ --version
g++ (Ubuntu 4.8.4-2ubuntu1
14.04) 4.8.4
Copyright (C) 2013 Free Software Foundation, Inc.

@noloader
Copy link
Collaborator

noloader commented Feb 1, 2016

You need to provide a buffer. This is not correct:

byte* digest; //resulting hash
SHA3 hash = SHA3(512);
...
hash.CalculateDigest(...);

Try:

byte digest[SHA3_512::DIGESTSIZE];
SHA3 hash = SHA3(512);
...
hash.CalculateDigest(...);

This is kind of odd:

 SHA3 hash = SHA3(512);

In C++, you usually just:

SHA3 hash(512);

Finally, I don't believe a bit-count with the SHA3 constructor is correct. I seem to recall it should be a byte-count. Also see sha3.h.

@noloader
Copy link
Collaborator

noloader commented Feb 1, 2016

This is already second submission of this but. First one was simply removed by someone without answering the question.

If you have questions, then you should discuss things at the User Group.

@GeorgeMatveev
Copy link
Author

Here's the result of your suggestion:

sha3.cpp:28:18: error: ‘DIGESTSIZE’ is not a member of ‘CryptoPP::SHA3’
byte digest [SHA3::DIGESTSIZE];

It is Generally good idea to return some error code from any function instead of void!
Also why method CalculateDigest throws no exception?
How we are supposed to debug it?
Cheers,
GM

@GeorgeMatveev
Copy link
Author

Your second idea:
SHA3 hash(512); compiles but returns:

$ ./sha3
Segmentation fault (core dumped)
:(

@noloader
Copy link
Collaborator

noloader commented Feb 1, 2016

Segmentation fault (core dumped)

Works for me. Here's the program:

void display(byte val)
{
    const char f = cout.fill('0');
    const streamsize w = cout.width(2);
    cout << std::hex << (unsigned int)(val);
    cout.width(w);
    cout.fill(f);
}

int main(int argc, char* argv[])
{
    try
    {
        SHA3_512 hash;
        string message = "Now is the time for all good men to come to the aide of their country";

        byte digest[SHA3_512::DIGESTSIZE];
        hash.CalculateDigest(digest, (const byte*) message.data(), message.length() );

        cout << "Digest: ";
        for_each(digest, digest+SHA3_512::DIGESTSIZE, display);
        cout << endl;
    }
    catch(const Exception& ex)
    {
        cerr << ex.what() << endl;
    }
    return 0;
}

Here's the result:

$ g++ -g2 -O2 -I . test.cxx ./libcryptopp.a -o test.exe
$ ./test.exe
Digest: 72bc1b2a40de023e8347fed5b7a5fc66f73dd74a143b49b2083f8f06df3c3025779752fa
fd7631fa2ecd85ec7cd0480c4512e96a4d61a21e5fe4a8e9bd8f97cf

It is Generally good idea to return some error code from any function instead of void!

OK, thanks. Crypto++ usually throws an exception rather than returning codes.

@noloader
Copy link
Collaborator

noloader commented Feb 1, 2016

We also update the documentation for member functions like CalculateDigest. They now include the precondition on the buffer sizes used for the digest. Also see Commit 389b6fc5da221680 and Commit 9430de2e46f5321e

@GeorgeMatveev
Copy link
Author

Glad to see that you are Responsive to library client developers!

I think it might be a Good Idea to update projects Wiki with simple and working examples like above.
Library API is NOT very intuitive as it is right now and could make life of even experienced C++ developers miserable.

Also linking with LOCAL ./libcryptopp.a makes LOTS of difference. Without it one would get: "error: ld returned 1 exit status". Which is not a good sign imho :-)

Thanks for helping us out with SHA-3 stuff!

@noloader
Copy link
Collaborator

noloader commented Feb 2, 2016

Glad to see that you are Responsive to library client developers!

Yes, its not a problem.

The reason I have not been providing answers here is I want to see where you go with things. I try to use it as a case study to identify gaps, like the missing wiki documentation on hashes. If I tell you to do X or don't do Y, then I influence the outcome of the experiment.

And I also experience the gaps when using other's warez. For example, this does not make sense to me: GCC missing libasan and libubsan after installing gcc and gcc-c++. I would have used it as authority to act in some way. I would not have closed the bug and forgotten about it.


I think it might be a Good Idea to update projects Wiki with simple and working examples like above.

Yes, you are right. We have good coverage on block ciphers and other areas, but it pretty much sucks for hashes. I'll put it on the TODO list.


Also linking with LOCAL ./libcryptopp.a makes LOTS of difference.

This one is tough because I know the gap exists, but I don't know how to disseminate the information.

When you found your compile and link command, where did you find it? Or did you just do what you kind of know?

@denisbider
Copy link
Collaborator

Respectfully, anyone who does:

byte* digest; //resulting hash
...
hash.CalculateDigest(digest, ...

... is not the target audience for Crypto++. A person who does something like this should not trusted with a C++ compiler to begin with.

Crypto++ has its problems - the design is somewhat overly arcane, it's attempting to be both a crypto library and an infrastructural framework. However, I'm not sure that the design of the library and its supporting materials should be aimed at C++ novices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants