Как стать автором
Обновить

Комментарии 60

причем тут ссылка, это взятие адреса
спасибо, давно не писал на С++, спутал с объявлением передачи параметра по ссылке
наверно потому, что на чистом С не прокатило бы использовать метод класса Read =)
Почему-то когда я читал эту заметку, мне было жалко программиста, потому, что такое ощущение, что его уже повесили уволили…
Вряд ли. Если за каждую опечатку в 1 символ увольнять людей — очень быстро никого не останется. Эта ошибка — повод внести изменения в корпоративные стандарты написания кода, повод начать использовать какой-нибудь валидатор исходников, повод настучать по ухам тестерам, чтобы обращали внимание на такие вещи. Увольнять человека — не повод.
Расскажите это придирчивым шефоменам.
дело в том, что это не C++, а C, на С++ так не пишут.
Как же оно раньше работало?
Как оно скомпилировалось?) Хотя pbArray было объявлено не как массив (указатель) если только.
Видимо это всеже указатель, судя по pb это указатель на BYTE. Если правильно трактовать венгерскую нотацию. Но тем не менее странно, что компилятор не стал матерится когда взятие по адресу сделали от указателя…
Подозреваю что тут злую шутку сыграл c-cast style в виде (void*). Компилятору то так-то все равно от чего указатель получать, если бы не было явного приведения типа, возможно он бы и ругнулся на несоответствие типов передаваемых параметров.
Других вариантов не вижу.
а мы не знаем, может компилятор и выдал ворнинг, который просто проигнорировали…
в статье по ссылке все написано, компилятор ничего не выдал и внутренние средства контроля тоже пропустили
int test(void* Var)
{
return 0;
}

int main(int argc, char* argv[])
{
char *pcArray;
pcArray=new char[10];
return test((void*)&pcArray);
}

воистину прокатывает ) Даже варнинга не выдал.
Мораль сей басни такова:
проверять все места где кастуется к (void *)
Мораль басни проще: пишешь — думай. А частные правила в каждом случае всё равно разные будут. Но вы правы: «не уверен — не кастуй».
Этот плакат должен висеть у каждого программера!
Просто не надо использовать C-style cast — он требует огромной аккуратности.
В данном случае даже если C-style cast заменить на static_cast — предупреждения всё равно не будет. А C-style cast да, по рукам бить за него.
НЛО прилетело и опубликовало эту надпись здесь
по моему это очевидно из топика:

Ошибка заключается в одном символе, вместо правильного кода:

hr = pStream->Read((void*)pbArray, (ULONG)cbSize, NULL);
* This source code was highlighted with Source Code Highlighter.

программист написал

hr = pStream->Read((void*)&pbArray, (ULONG)cbSize, NULL);
НЛО прилетело и опубликовало эту надпись здесь
Ну на самом деле от этого никак не скроешься, налицо человеческий фактор. Я уже писал, что человек просто пытался избавится от ошибки, там компилятор не позволил бы сделать неявное приведение типов, еслиб он не поставил (void*). Тут простое избежание ошибки, чтоб просто откомпилировать код. Человек либо вообще не понимал что он делает, либо понимал но почему-то сделал. В общем защитится от этого никак нельзя. Единственный способ ИМХО это юнит-тестирование программных модулей, которые нечасто используются в программе. Но это тоже не панацея от всех бед. Странно что вообще программа работала. видимо кусок либо стоял между ifdef долгое время либо просто в крайне редко выполняемом условии. Потому что любое выполнение подобного кода приведет к сегфолту.
Может быть существуют устойчивые к ошибкам способы приведения типов или программные средства тестирования подобного кода?
За меня уже ответили, но я дополню. Ошибку надо исправлять в проектировании. IStream::Read принимает void*, а должен BYTE*. Если бы он принимал BYTE* никаких кастов не нужно было бы и компилятор выдал бы ошибку.
Другое дело, что COM-интерфейсы менять нельзя. Поэтому эту ошибку в проектировании (и многие другие) уже не исправишь.
А статические анализаторы кода типа lint с таким справятся?
Мораль сей басни такова:
использовать void* только если возле вашего лица раскаленный утюг… (по возможности, конечно)
А, оказалось, в данном случае от такого каста никуда не деться… :(
Я вот ну ни строчки не понял :) как меня сюда занесло :)
Компилятор странный такой — полагает, что раз ты явно приводишь тип к void*, то ты знаешь, что делаешь.

Среди каментов не нашел только мысли, что программисту думать нужно. И таких ошибок не делать. И не грешить на то, что компилятор не выдал ворнинга: а то придумали защиту от дурака, защиту от защиты от дурака и т.д.

И всё равно ошибки ловим. А всё почему? Потому что кодер протупил. В данном случае — то ли опечатался, то ли не представлял, что пишет.
Скорее второе. Тут налицо избавление от ошибки, которая выдается если явно не преобразовывать тип. Но мне кажется даже маломальски программирующий человек разберется в чем проблема, а не начнет городить такое. В общем будем вновь уповать на индусов…
По идее, упасть должно или некорректные данные прочитать из стека или из кучи.
Наверное, код был редко выполняющимся. И все равно непонятно, как программер, писавший это, не удосужился протестировать метод оО. Неужели белиберда, прочитанная из стрима, настолько была похожа на правду? )
Вот что я не понимаю. Уже сколько лет и процы и оси поддерживают бит исполнения для страницы, какая ошибка переполнения буфера?? Покрешится да и только.
Падение тоже можно считать уязвимостью к DoS
В атрибутах той секции данных вполне могли быть установены биты и для чтения-записи, и для выполнения.
По логике мало вероятно. Станицы с текстом не должны быть доступны для данных.
Здесь, скорее наоборот, если адрес переполняемого буфера находился в куче, тогда не в .text перезаписываются данные, а в .data — шеллкод. И страница .data может оказаться не защищенной от выполнения в ней кода.
Ура! Microsoft публикует исходные коды! :)
Да уж. Лучшеб не публиковал, а то жутковато становится…
Диалог оптимиста с пессимистом. :-)
Вами приведён код практически полностью на С. Со всеми проблемами, присущими коду на С (я насчитал как минимум четыре проблемы в одной этой строке). В коде не используются преимущества С++, это и явилось источником проблем.

Вменяемые C++ фреймворки дают возможность писать такой код:

Array<byte> data;
Stream<byte> stream;
...
stream.Read(data, dataSize);


* This source code was highlighted with Source Code Highlighter.
1) со всеми вытекающими проблемами менеджмента / фрагментации хипы
2) внутри у них такой же код
По первому согласен. По второму не исключено. Мне уже становится страшно от написанного MS. Кидайте в меня камни, но на этот раз страх объективен.
Ух какой-то MS фан наминусовал, мб пояснит за что?
те, кому нечего сказать — минусуют =)
НЛО прилетело и опубликовало эту надпись здесь
Тоже был удивлен, как это код вообще мог работать и был ли он рабочим.
Код действительно плохо тестировали, т.к. вероятность того что после выполнение этой строки не будет ошибки очень мала.
Да что тут говорить — ошибка случилась потому, что использованный фреймворк по стилю программирования устарел лет на 15. Пользоваться такими библиотеками, где для выполнения нужной операции приходится писать конструкции типа (void*)&pbArray, я бы не посоветовал никому и никогда. Это ведь слабая типизация, по нынешним понятиям стилистики С++ — почти что смертный грех.

И то, что они предлагают — более мощные инструменты, более тщательный анализ — лишь лечение следствия, а не причины.
НЛО прилетело и опубликовало эту надпись здесь
> С++ не для дубов и требует понятия того, что ты пишешь и куда подставлешь

Прозреваю С++фага. Отсутствие type safety является проблемой и Си, и Си++ (а только потом уже тех, кто вынужден ими пользоваться), точка.
НЛО прилетело и опубликовало эту надпись здесь
Вот поэтому в билд-скриптах developer.yahoo.com/geo и в частности developer.yahoo.com/geo/placemaker стоит "-Wall -Werror" навеки, хотя их все дружно люто бешено ненавидят, потому что придирается к каждой строчке. Но таких ошибок компилятор не пропустит.
-Wall -Werror не поможет в данном конкретном случае, писали уже выше.
(void*)&pbArray даст using old-style casts warning. Этого уже достаточно будет чтобы посмотреть на код более пристально, и с большой вероятностью когда программист будет переписывать его на static_cast<void*> он эту ошибку увидит.
Цигвиновский g++ 4.3.2 варнинги не показывает, я потому и написал так. Попробую найти более позднюю версию.
Я тоже нашёл опечатку в один символ :)
последняя строка статьи:
«пожно почитать в блоге»
должно быть
«можно почитать в блоге»
спасибо, надеюсь хабр из-за нее не сломается :-)
думаю, зло все же в приведении к (void*)…
Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации