Home

Previous Entry | Next Entry

yellow eyes, snake bells, Speaker Rabbit, Борщ с гренкой
4 апреля центр CERT, занимающийся координацией обменом информацией о компьютерных уязвимостях между экспертами по безопасности опубликовал заметку о уязвимости в новых версиях (4.2+) GNU C Compiler, из-за которой определенные проверки на переполнение буферов памяти оптимизируются при компиляции приложения как ненужные. Связано это с тем, что в стандарте языка C определено, что при увеличении указателя на константу получающийся указатель должен указывать либо на первоначальный объект, либо на объект следом за первоначальным. То есть, фактически, предполагается, что операция сложения указателя и константы неубывающая. В таком случае код

uint_t *x, *foo = some_object();
uint_t i = 0, len = some_object_length();

for (x=foo; x < foo+len; x++) {
     i++;
}

становится просто ненужным, ведь указанное условие и так выполняется согласно требованию стандарта языка C, а значит, можно оптимизировать этот цикл и развернуть его в последовательность из len операций i++, которую тоже можно соптимизировать. Обратное неверно и, более того, в стандарте неопределено.

Реально такое изменение поведения компилятора правильно, оно приводит компилятор в соответствие со стандартом, и команда разработчиков gcc не собирается его "откатывать". Но в то же время оно фактически устраняет существующие во многих приложениях проверки на переполнение буферов. И эти проверки (неправильные с точки зрения стандарта C) необходимо переписывать.

Замечу, что изменение в поведении касается только проверок указателей. Это тот случай, когда тип сравниваемого имеет значение. Проверки на то, что сумма указателя и константы по-прежнему больше самого указателя, то есть, получившийся указатель не вышел за границу разрядности (и тем самым старший бит был потерян, а мы "завернулись" в начало адресного пространства), неверны сами по смыслу, ведь в таком случае все равно надо проверять не на "оборачиваемость" указателя, а на превышение размеров буфера. Фактически, изначальная проверка была неверной и работала только потому, что компилятор ее "не замечал".

В Samba подобная проблема существует, если ее собрать 32-битной версией gcc 4.2+ для тех фрагментов, где мы имеем входящий пакет, в котором указаны блоки смещением внутри пакета и размером блока от указанного смещения. Текущий код выглядит вот так:

char *buff_base;
uint32_t in_offset, in_size;

       if (buff_base + in_offset + in_size < buf_base)
               goto error;

Прямая замена на сравнение in_offset+in_size < alloc_size, где uint32_t alloc_size -- размер буфера, в который мы копируем данные из пакета, на самом деле не эквивалентна оригинальной проверке. Полная проверка будет выглядеть вот так:

   if (in_offset > alloced_size || in_size > alloced_size ||
                       in_offset + in_size > alloced_size ||
                       in_offset + in_size < in_offset )
               goto error;

То есть, количество проверок увеличилось, но это плата за понимание стандарта языка. Этот код будет правильно скомпилирован любой версией gcc.

Tags:

Comments

[info]poige wrote:
Apr. 9th, 2008 07:59 am (UTC)
По-моему, эта «полная проверка», как-раз, подвержена ...
[info]abbra wrote:
Apr. 9th, 2008 08:09 am (UTC)
Re: По-моему, эта «полная проверка», как-раз, подвержена .
Нет, я же говорил -- тип имеет значение. В твоем случае тип размера должен быть unsigned int (long), а не unsigned char. В крайнем случае, size_t, вообщем, максимальным на архитектуре . Тогда все проверки будут верными.

В случае 2^N мы имеем "обертку" в unsigned int, при которой sz_offset+sz_size превысят 2^N и всегда будут меньше sz_allocated. Предлагаю подумать почему. :-)
[info]poige wrote:
Apr. 9th, 2008 08:26 am (UTC)
Тип, действительно, имеет значение, потому и выбрал byte.
Для демонстрации с обычной integer-размерностью, буфер был бы немного большим. )
[info]abbra wrote:
Apr. 9th, 2008 10:16 am (UTC)
Re: Тип, действительно, имеет значение, потому и выбрал by
Ага, я не дописал одну проверку:
in_offset+in_size < in_offset

Собственно, это можно видеть в коммите http://git.samba.org/?p=samba.git;a=commitdiff;h=09852899cadc48abe2f2651ecbceaf881198e648;hp=a4e3bc2bade8bf74696e1c6ced74da563ff2df7b
там соответствующая проверка выполняется до проверок на переполнение буфера.

Edited at 2008-04-09 10:19 am (UTC)
[info]poige wrote:
Apr. 9th, 2008 10:23 am (UTC)
> не дописал одну проверку
Вот-вот.
[info]buldozr wrote:
Apr. 9th, 2008 10:11 pm (UTC)
Я всегда полагал, что использовать операции порядкового сравнения над указателями - дурной тон.

Кстати, ваш код для полной проверки тоже не эквивалентен первоначальному: в первом не проверяется выход за alloced_size без 32-битного переполнения.
[info]abbra wrote:
Apr. 10th, 2008 03:09 am (UTC)
Полную картину можно увидеть вот в этом коммите:
http://gitweb.samba.org/?p=samba.git;a=commitdiff;h=09852899cadc48abe2f2651ecbceaf881198e648

Но вообще, порядковое сравнение указателей мне тоже всегда было как-то не по себе.