возможная уязвимость переполнения буфера для va_list в C? - PullRequest
4 голосов
/ 27 августа 2011

У меня есть следующий код:

int ircsocket_print(char *message, ...)
{
    char buffer[512];
    int iError;
    va_list va;
    va_start(va, message);
    vsprintf(buffer, message, va);
    va_end(va);
    send(ircsocket_connection, buffer, strlen(buffer), 0);
    return 1;
}

И я хотел бы знать, является ли этот код уязвимым для переполнения буфера путем предоставления массивов символов размером> 512 в список переменных?И если так - Как я могу это исправить?

спасибо.

Ответы [ 4 ]

10 голосов
/ 27 августа 2011

Да, это уязвимо.

Вы можете реализовать свою функцию следующим образом:

int ircsocket_print(char *message, ...)
{
    char buf[512];
    char *buffer;
    int len;
    va_list va;

    buffer = buf;
    va_start(va, message);
    len = vsnprintf(buffer, 512, message, va);
    va_end(va);

    if (len >= 512)
    {
        buffer = (char*)malloc(len + 1);
        va_start(va, message);
        len = vsnprintf(buffer, len + 1, message, va);
        va_end(va);
    }

    send(ircsocket_connection, buffer, len, 0);

    if (buffer != buf)
        free(buffer);
    return 1;
}
8 голосов
/ 27 августа 2011

Да, это уязвимо.

Просто используйте vsnprintf вместо:

vsnprintf(buffer, sizeof(buffer), message, va);
6 голосов
/ 27 августа 2011

Как и все остальные, основной ответ на ваш вопрос: «Да, вы уязвимы для переполнения буфера».

В C99 вы можете использовать VLA:

void ircsocket_print(const char *fmt, ...)
{
    va_list args;

    va_start(args, fmt);
    int len = vsnprintf(0, 0, message, args);
    va_end(args);

    char buffer[len+1];

    va_start(args, fmt);
    int len = vsnprintf(buffer, len+1, message, args);
    va_end(args);

    send(ircsocket_connection, buffer, len, 0);
}

Обратите внимание на идиому, которая вызывает vsnprintf() один раз с длиной 0 (второй ноль), чтобы получить требуемую длину, а затем вызывает ее во второй раз для форматирования данных в буфер.Также обратите внимание на тщательный сброс args после каждого вызова на vsnprintf();что требуется стандартом C:

§7.15 <stdarg.h>

Если требуется доступ к переменным аргументам, вызываемая функция должна объявить объект (обычно называемый ap в этом подпункте) типа va_list.Объект ap может быть передан в качестве аргумента другой функции;если эта функция вызывает макрос va_arg с параметром ap, значение ap в вызывающей функции является неопределенным и должно быть передано макросу va_end перед любой дальнейшей ссылкой на ap.

Одним из недостатков этой формулировки является то, что она принимает пессимистический взгляд и безоговорочно вызывает vsnprintf() дважды.Вы можете предпочесть оптимистичный подход (в большинстве случаев достаточно 512) и выделять больше места только в том случае, если первый вызов показывает, что этого недостаточно.

Еще один недостаток использования VLAнапример, если у вас не хватает места для локальной переменной buffer, ваш код может никогда не получить возможности для восстановления.Вы должны судить, насколько это серьезная проблема.Если это проблема, используйте вместо этого явное выделение памяти (malloc()):

char buffer = malloc(len+1);
if (buffer == 0)
    return;  // Report error?
...second vsnprintf() and send()...
free(buffer);

Поскольку ваша функция когда-либо возвращала только константу 1, нет очевидной причины делать ее функцией, возвращающей что-либо- если это функция, возвращающая void, вызывающему коду не требуется проверка возвращаемого значения.OTOH, может быть, вы должны возвращать результат вызова send() (и, возможно, сообщение об ошибке, если malloc() не удается).

Я также сделал параметр формата (сообщения) в const char *;ни эта функция, ни те, которые она вызывает, не изменяют строку формата.

2 голосов
/ 27 августа 2011

Если вы работаете в Linux, вы можете использовать vasprintf(), который выделит буфер правильного размера.

Если вам нужна переносимость, вы можете использовать vsnprintf(), чтобы избежать переполнения буфера.

...