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

AES GCM encryption with large tag size results in incorrect output, out-of-bounds reads #954

Closed
guidovranken opened this issue Jun 8, 2020 · 2 comments
Labels

Comments

@guidovranken
Copy link

The code below is essentially the same as the example at https://www.cryptopp.com/wiki/GCM_Mode#AEAD except that the tag size is configurable using a compiler define.

The following anomalies were observed with fuzz testing:

  • Compile with -DTAG_SIZE=32 to observe that encryption succeeds, but subsequent decryption does not
  • Compile with -DTAG_SIZE=200 to observe out-of-bounds reads (use Valgrind or AddressSanitizer)
#include <cassert>
#include <gcm.h>
#include <aes.h>
#include <iostream>
#include <filters.h>
using namespace std;
using namespace CryptoPP;

int main(void)
{
    byte key[32]; memset( key, 0, sizeof(key) );
    byte iv[12]; memset( iv, 0, sizeof(iv) );

    string adata( 16, (char)0x00 );
    string pdata( 16, (char)0x00 );

    // Encrypted, with Tag
    string cipher, encoded;

    // Recovered (decrypted)
    string radata, rpdata;

    /*********************************\
      \*********************************/

    try
    {
        GCM< AES >::Encryption e;
        e.SetKeyWithIV( key, sizeof(key), iv, sizeof(iv) );

        // AuthenticatedEncryptionFilter defines two
        //   channels: DEFAULT_CHANNEL and AAD_CHANNEL
        //   DEFAULT_CHANNEL is encrypted and authenticated
        //   AAD_CHANNEL is authenticated
        AuthenticatedEncryptionFilter ef( e,
                new StringSink( cipher ), false,
                TAG_SIZE /* MAC_AT_END */
                ); // AuthenticatedEncryptionFilter

        // Authenticated data *must* be pushed before
        //  Confidential/Authenticated data. Otherwise
        //  we must catch the BadState exception
        ef.ChannelPut( AAD_CHANNEL, (const unsigned char*)adata.data(), adata.size() );
        ef.ChannelMessageEnd(AAD_CHANNEL);

        // Confidential data comes after authenticated data.
        // This is a limitation due to CCM mode, not GCM mode.
        ef.ChannelPut( DEFAULT_CHANNEL, (const unsigned char*)pdata.data(), pdata.size() );
        ef.ChannelMessageEnd(DEFAULT_CHANNEL);
        printf("Encryption succeeded\n");
    }
    catch( CryptoPP::Exception& e )
    {
        cerr << "Caught Exception..." << endl;
        cerr << e.what() << endl;
        cerr << endl;
        return 0;
    }


    try
    {
        GCM< AES >::Decryption d;
        d.SetKeyWithIV( key, sizeof(key), iv, sizeof(iv) );

        // Break the cipher text out into it's
        //  components: Encrypted and MAC
        string enc = cipher.substr( 0, cipher.length()-TAG_SIZE );
        string mac = cipher.substr( cipher.length()-TAG_SIZE );

        // Sanity checks
        assert( cipher.size() == enc.size() + mac.size() );
        assert( enc.size() == pdata.size() );
        assert( TAG_SIZE == mac.size() );

        // Not recovered - sent via clear channel
        radata = adata;

        // Object *will* throw an exception
        //  during decryption\verification _if_
        //  verification fails.
        AuthenticatedDecryptionFilter df( d, NULL,
                AuthenticatedDecryptionFilter::MAC_AT_BEGIN | AuthenticatedDecryptionFilter::THROW_EXCEPTION, TAG_SIZE );

        // The order of the following calls are important
        df.ChannelPut( DEFAULT_CHANNEL, (const unsigned char*)mac.data(), mac.size() );
        df.ChannelPut( AAD_CHANNEL, (const unsigned char*)adata.data(), adata.size() );
        df.ChannelPut( DEFAULT_CHANNEL, (const unsigned char*)enc.data(), enc.size() );

        // If the object throws, it will most likely occur
        //   during ChannelMessageEnd()
        df.ChannelMessageEnd( AAD_CHANNEL );
        df.ChannelMessageEnd( DEFAULT_CHANNEL );

        // If the object does not throw, here's the only
        //  opportunity to check the data's integrity
        bool b = false;
        b = df.GetLastResult();
        assert( true == b );

        // Remove data from channel
        string retrieved;
        size_t n = (size_t)-1;

        // Plain text recovered from enc.data()
        df.SetRetrievalChannel( DEFAULT_CHANNEL );
        n = (size_t)df.MaxRetrievable();
        retrieved.resize( n );

        if( n > 0 ) { df.Get( (byte*)retrieved.data(), n ); }
        rpdata = retrieved;
        assert( rpdata == pdata );

        // All is well - work with data
        cout << "Decrypted and Verified data. Ready for use." << endl;
        cout << endl;

        cout << "adata length: " << adata.size() << endl;
        cout << "pdata length: " << pdata.size() << endl;
        cout << endl;

        cout << "recovered adata length: " << radata.size() << endl;
        cout << "recovered pdata length: " << rpdata.size() << endl;
        cout << endl;
    }
    catch( CryptoPP::Exception& e )
    {
        cerr << "Caught Exception..." << endl;
        cerr << e.what() << endl;
        cerr << endl;
        return 0;
    }
    return 0;
}
@noloader
Copy link
Collaborator

noloader commented Jul 7, 2020

-DTAG_SIZE=32 and -DTAG_SIZE=200 ...

The tag size is limited to the cipher's block size, which is 16 for AES.

@noloader
Copy link
Collaborator

noloader commented Jul 7, 2020

Cleared at Commit a7b32867ddf8. The GCM encryptor or decryptor will throw if the tag size is greater then the block size.

@noloader noloader closed this as completed Jul 7, 2020
@noloader noloader added the Bug label Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants