Link to home
Start Free TrialLog in
Avatar of CSecurity
CSecurityFlag for Iran, Islamic Republic of

asked on

FormatMessage Error

Hi

What's wrong with this code:

void PrintMessage()
{

DWORD_PTR args = (DWORD_PTR)("Test1");

LPTSTR pszResult;
char fmt[MAX_PATH];
strcpy(fmt, "Name: %1!s!\r\n\r\n");

va_list marker;
va_start(marker, "Test11");

FormatMessage(
        FORMAT_MESSAGE_ALLOCATE_BUFFER |
        FORMAT_MESSAGE_FROM_STRING,
        (LPTSTR)(fmt),
        0,
        0,
        (LPTSTR) &pszResult,
        10000,
        &marker);

MessageBoxA(0, pszResult, "Result", 64);

}

I get access denied error, you cannot change first parameter. So tell me resoulution without changing first parameter.

Thanks
Avatar of evilrix
evilrix
Flag of United Kingdom of Great Britain and Northern Ireland image

parameter 6 shoudld be just pszResult not  (LPTSTR) &pszResult.

lpBuffer [out]
A pointer to a buffer that receives the null-terminated string that specifies the formatted message. If dwFlags includes FORMAT_MESSAGE_ALLOCATE_BUFFER, the function allocates a buffer using the LocalAlloc function, and places the pointer to the buffer at the address specified in lpBuffer.

This buffer cannot be larger than 64K bytes.
Avatar of js-profi
js-profi

you dont pass the &marker but as many arguments the format statement requires. for example if you have "%s %d %s" as format specifiers, you would pass a (const) char pointer, an int and a further (const char) pointer. the va_list is used in the called function.
Here the working code:
#include <Windows.h>
#include <Tchar.h>

void PrintMessage()
{

//DWORD_PTR args = (DWORD_PTR)("Test1");

TCHAR pszResult[ 10000 ];
memset( pszResult, 0, 10000*sizeof(TCHAR) );
TCHAR fmt[MAX_PATH];
_tcscpy(fmt, TEXT("Name: %1!s!\\r\\n\\r\\n\\0") );

va_list marker;
va_start(marker, TEXT("\\0") );

FormatMessage(
        //FORMAT_MESSAGE_ALLOCATE_BUFFER |
        FORMAT_MESSAGE_FROM_STRING,
        (LPTSTR)(fmt),
        0,
        0,
        pszResult,
        10000,
        &marker);

MessageBox(0, pszResult, TEXT("Result"), 64);
}

// TCHAR <=> char or wchar_t (if UNICODE defined).
Avatar of CSecurity

ASKER

Evilrix solution didn't worked, still error 5

js-profi's solution wasn't clear, I don't know what to change.

pchela's solution wouldn't work, as I said I'm not allowed to change first parameter. It's a special situtation.

Thanks
>> Evilrix solution didn't worked
I never said it was a solution... I was just pointing out you are passing this parameter incorrectly. :)

Maybe your general usage of this function needs to be reviewed. I'd strongly suggest reading the MSDN page carefully and checking and double checking all the parameters you are passing are correct.

This is an odd error for that function, though. Are you sure FormatMessage is generating it?

BTW: Since you are getting FormatMessage to allocate memory you also need to free it, right?
below is sample code from MSDN. i was wrong with the va_list * argument which indeed is a variable number of arguments like in printf in the first case. but when calling FormatMessage it is expected you already have retrieved the arguments by a va_list * what means you already resolved the ... arguments. alternatively you can pass the arguments in a void* array where you put the pointers of the strings to replace into. the number of arguments expected is determined by the format string (%1, %2, ...). you tell the FormatMessage that you passed an array by specifying the FORMAT_MESSAGE_ARGUMENT_ARRAY option.
// InitRsrcDlg -- set up the dialog contents for the
// dialog box represented by hWnd.

void InitRsrcDlg(HWND hWnd)
{
   TCHAR szTemplate[256];

   TCHAR szDate[32];
   TCHAR szTime[32];

   PVOID pvBuffer;

   PVOID pvArgs[] = {szTime, szDate};

   TCHAR szCurrency[MAX_CURRENCY_LEN];
   TCHAR szNumber[MAX_NUMERIC_LEN];

   // First, initialize the startup time string.

   // GetDateFormat is the NLS routine that formats a time in a 
   // locale-sensitive fashion.  In this case, the current date is 
   // being displayed in the long date format
   if (0 == GetDateFormat(g_dwCurLcid, DATE_LONGDATE, NULL, NULL,
      szDate, 32)) {
      return;
   }

   // GetTimeFormat is the NLS routine that formats a time in a 
   // locale-sensitive fashion.  In this case, the current time is 
   // being displayed in the default format
   if (0 == GetTimeFormat(g_dwCurLcid, 0, NULL, NULL, szTime, 32)) {
      return;
   }

   // Load the formatting string for the date and time entry from the 
   // satellite DLL, not the main program.  This resource is in 
   // whatever language is represented in the resource DLL; it is not 
   // directly sensitive to the operational language.
   if (0 == LoadString(g_hRsrc, IDS_THETIMEIS, szTemplate, 256)) {
      return;
   }

   // Use FormatMessage to create a single string which contains these
   // items in the correct order.  FormatMessage will allocate the
   // output buffer containing the formatted message itself, but
   // we will be responsible for freeing it when we're done.
   FormatMessage(FORMAT_MESSAGE_FROM_STRING | 
      FORMAT_MESSAGE_ALLOCATE_BUFFER | 
      FORMAT_MESSAGE_ARGUMENT_ARRAY,
      szTemplate,
      0,   // Ignored
      0,   // Ignored
      (LPTSTR) &pvBuffer,
      256,
      (va_list *) pvArgs);

   // Display the string in the dialog box.
   SetDlgItemText(hWnd, IDC_DATETIME, (LPCTSTR) pvBuffer);

   // Release the buffer.
   LocalFree(pvBuffer);

   // Now, display the currency amount and the number using NLS.
   // GetCurrencyFormat is the NLS routine that formats a currency 
   // amount in a locale-sensitive fashion.
   if (0 != GetCurrencyFormat(g_dwCurLcid, 0, k_szCurrencyAmount, NULL,
      szCurrency, MAX_CURRENCY_LEN)) {
      SetDlgItemText(hWnd, IDC_CURRENCY, szCurrency);
   }

   // GetNumberFormat is the NLS routine which formats a number in a 
   // locale-sensitive fashion.
   if (0 != GetNumberFormat(g_dwCurLcid, 0, k_szNumericAmount, NULL, 
      szNumber, MAX_NUMERIC_LEN)) {
      SetDlgItemText(hWnd, IDC_NUMBER, szNumber);
   }
}

Open in new window

js-profi, as I said, FIRST PARAMETER SHOULD BE AS I SAID

FORMAT_MESSAGE_FROM_STRING |  FORMAT_MESSAGE_ALLOCATE_BUFFER

Which equals 500 in hex. It's a special case, I'm investigating something.

Thanks
a valid va_list you get by calling your PrintMessage function with the following signature:

void PrintMessage(LPCTSTR str, ...)
{
     ...
     va_list marker;
     va_start(marker, str);
     // now the va_list is valid and contains str plus all other arguments passed when calling PrintMessage
     FormatMessage(
        FORMAT_MESSAGE_ALLOCATE_BUFFER |
        FORMAT_MESSAGE_FROM_STRING,
        (LPTSTR)(fmt),
        0,
        0,
        (LPTSTR) &pszResult,
        10000,
        &marker);
       ...  
}

now the marker should be valid. if you dont want to change arguments of PrintMessage you could try calling a helper function with variable arguments and return a pointer to the marker you rtrieved in that helper. the problem i see with this is that the va_arg will retrieve its values from stack. but when you called a function and it returned  the stack was rewinded and the args were not more available.

by the way, you specified FORMAT_MESSAGE_ALLOCATE_BUFFER but pass a buffer of 10000. that doesnt fit together. you might pass a pointer but do not allocate storage and pass 0 for the size argument.
Let me tell you what I'm doing and you tell me is it possible or not.

I'm investigating a vulnerability in an application, it calls FormatMessage without any check, it calls like this:

FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
        FORMAT_MESSAGE_FROM_STRING,
..
...
..
..
)

Format is something like this: Output %1 and %2

I control what to be passed in %1 and %2, I can crash the program by something which can't be Formatted, like FF FF FF FF FF

I want to know is it possible to have stack overflow or Format String vulnerability,

Now do you have anything to say here?

Thanks
ASKER CERTIFIED SOLUTION
Avatar of js-profi
js-profi

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
as you specified FORMAT_MESSAGE_ALLOCATE_BUFFER  the FormatMessage should be safe regarding buffer overflow. it would check the string length of the arguments given and if not crashing because of a wrong pointer it would allocate enough memory to copy the string or throw a memory exception. but it wouldnt do an overflow. i said "should be safe" cause you passed a buffer of 10000 chars what isnt necessary with FORMAT_MESSAGE_ALLOCATE_BUFFER . nevertheless as the size of the buffer is passed also teh FormatMessage would not write beyond buffer size.
So in my case, it have something like this:
retcode = FormatMessage(...)

Which I can't control format which is "Output is %1 and %2", but I control what is passed and to be filled in %1 and %2. I sent FF FF FF FF and it's UNicode by the way.

It returns and error code in retcode. Developer wrote a code like this after call to FormatMessage:

if retcode <> 0 ThrowException And Terminate Process with C Lib

By the crash I meant this, but it's important too, I crashed a popular program, anyways...

So you say stack overflow or format string vulnerability is not a case here, right?

Thanks
If I pass buffer larger than 10000 should case stack/buffer overflow?
no, you only pass pointers via the stack. if the text to replace the placeholder would require a buffer greater than 10000 the FormatMessage would allocate own memory.
So in a word you say it's impossible to have format string or stack/buffer overflow, right?
yes, thats my opinion. but you should care for the wrong pointers passed.
What do you mean exactly by wrong pointers in my case?
i mean the "FF FF FF FF ".
FF FF FF FF is what I pass to FormatMessage and cause FOrmatMessage to return error code
yes, i misunderstood that you _had_ a problem with 0xffffffff pointer. but i now understand that you only tested the function.  
Thanks