0

I have an error logging mechanism, which constructs a buffer using vsntprintf_s. Unfortunately it so happens that if it sees a "%" symbol, then there is an exception while constructing the buffer.

TCHAR szBuffer[2000];
    if (lpszFormat !=  NULL)
        _vsntprintf_s(szBuffer, _countof(szBuffer), lpszFormat, args);

Whole function -

bool someFunction::TraceLog (LPCTSTR lpszFormat, ...)
{

    va_list args = nullptr;
    va_start (args, lpszFormat);

TCHAR szBuffer[2000];
    if (lpszFormat !=  NULL)
        _vsntprintf_s(szBuffer, _countof(szBuffer), lpszFormat, args);
    else
        _tcsncpy_s (szBuffer, _T("NULL format for TraceGen"), _TRUNCATE);
}

where, if the input string, lpszFormat contains a '%' it fails. The "%" is not meant to be an operator, but is rather for something within the string itself. E.g. Test%Test

What can be the best way to handle this?

user1173240
  • 1,455
  • 2
  • 23
  • 50
  • I don't know what is `_vsntprintf_s`, but, as with any formatting functions, isn't `lpszFormat` _expected_ to have (unescaped or whetever) percent signs match the number of arguments in `args`? Where does your `lpszFormat` string come from? – Petr Oct 01 '15 at 10:12
  • Are you sure about `args`? – LPs Oct 01 '15 at 10:12
  • `va_list args = nullptr; va_start (args, lpszFormat);` is how the args is retrieved. The `lpszFormat` string comes from another function. – user1173240 Oct 01 '15 at 10:17
  • 1
    Is `lpszFormat` the last variable before `...` in your function declaration? Better: post the whole function. – LPs Oct 01 '15 at 10:25
  • `sprintf` itself is dangerous (some compilers or static code analysis tools therefor check for correctness), even without getting it from the outside. [Validate the format string](http://stackoverflow.com/questions/5301357/how-to-validate-user-supplied-printf-format-string-against-parameter-list) and use a [safer implementation](http://stackoverflow.com/questions/11008891/boostformat-vs-sprintf). – Florian Oct 01 '15 at 10:38
  • 4
    if you are using "Test%Test" as a format to something like printf then you are doomed. You shouldn't use user supplied strings as parameters to sprintf. – Tom Tanner Oct 01 '15 at 10:44
  • What would be the best way to sanitize the string, and remove such offending characters which can cause format specifier errors? – user1173240 Oct 01 '15 at 11:33
  • 1
    If you want a single `%` in the string, you can use 2 of them like `%%`. – Weather Vane Oct 01 '15 at 12:08
  • @WeatherVane - but then you'd have to write an entire `printf` format parser to figure out if the % chars you find are valid, and then compare them to the arguments, and then hope that arguments to, say, a `%s` format specifier actually point to valid memory anyway. – Andrew Henle Oct 01 '15 at 12:12
  • I agree with @AndrewHenle. And after a little research I was not surprised to find those format parsers already have been written for all kind of use case - and without [pointer validations](http://stackoverflow.com/questions/551069/testing-pointers-for-validity-c-c): see e.g. slashmais's answer [here](http://stackoverflow.com/a/3742999/4763489). – Florian Oct 01 '15 at 19:58
  • 1
    And for all practical purposes, if you document a function call as using `printf()`-style format and arguments and a C programmer doesn't do that properly, there's not much you can do to fix the problem, as it's located between the keyboard and chair and not in the code. This is a *de facto* "botched `printf()` argument" question, and I doubt I'd find any answers on this site that would recommend reimplementing `printf()` in a way that handles botched formats. – Andrew Henle Oct 01 '15 at 20:05
  • @user1173240 if my answer is the accepted answer for the question then you should accept it by clicking the hollow check mark next to the answer, so that it becomes green. If you found a different solution then describe it as a new answer and accept that one. – Dialecticus Oct 03 '15 at 09:36
  • @Dialecticus - Sure. I do not have access to internet during the weekends, so I could not do it. Thanks for your answer. I have decided to modify the input string format. – user1173240 Oct 05 '15 at 07:40

1 Answers1

1

The best way to handle this is to always have format string under your control (and by you I mean the code that you write). You must not have a format string like "Test%Test", because that is against the rules for a format string.

If you want to print that exact string then corresponding format string should be "Test%%Test".

If the contents of the string is not under your control then the format string should just be "%s" and the actual string should be given as function's next parameter.

Dialecticus
  • 16,400
  • 7
  • 43
  • 103