Golded SIGABRT

[Nil A]

Hello, All!

Я имею устойчивый репродакшен баг, на голдеде, построенном из гитхаб мастер.

Program received signal SIGABRT, Aborted.
gdb) bt
#0 0x00007ffff7099c37 in __GI_raise (sig=***@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1 0x00007ffff709d028 in __GI_abort () at abort.c:89
#2 0x00007ffff70d62a4 in __libc_message (do_abort=***@entry=2,
fmt=***@entry=0x7ffff71e5db0 "*** %s ***: %s terminated\n") at
../sysdeps/posix/libc_fatal.c:175
#3 0x00007ffff717187c in __GI___fortify_fail (msg=<optimized out>,
***@entry=0x7ffff71e5d47 "buffer overflow detected") at fortify_fail.c:38
#4 0x00007ffff7170750 in __GI___chk_fail () at chk_fail.c:28
#5 0x00007ffff716f8eb in __strcat_chk (
dest=***@entry=0xd20488 "\351 \323\324\301\314\311
\304\322\301\324\330\323\321 \351\314\330\321 \355\325\322\317\315\305\303
\323\317 \332\315\305\305\315 \347\317\322\331\316\331\336\305\315 \311
\317\324\322\325\302\311\314 \355\325\322\317\315\305\303 \317\304\316\325
\307\317\314\317\327\325 \372\315\305\300. \356\317
\327\331\322\317\323\314\317 \3257
#6 0x000000000047dad6 in strcat (__src=<optimized out>,
__dest=0xd20488 "\351 \323\324\301\314\311 \304\322\301\324\330\323\321
\351\314\330\321 \355\325\322\317\315\305\303 \323\317 \332\315\305\305\315
\347\317\322\331\316\331\336\305\315 \311 \317\324\322\325\302\311\314
\355\325\322\317\315\305\303 \317\304\316\325 \307\317\314\317\327\325
\372\315\305\300. \356\317 \327\331\322\317\323\314\317 \325 \324\3172
#7 ScanKludges (msg=***@entry=0xd1ebec, getvalue=***@entry=1) at
/home/fido/src/golded-plus/golded3/geline.cpp:1743
#8 0x000000000047fe80 in MakeLineIndex (msg=0xd1ebec, margin=141,
getvalue=true, header_recode=<optimized out>) at
/home/fido/src/golded-plus/golded3/geline.cpp:3071
#9 0x00000000004817e9 in Area::LoadMsg (this=***@entry=0xbab570,
msg=***@entry=0xd1ebec, msgno=8388, margin=***@entry=141, mode=***@entry=0)
at /home/fido/src/golded-plus/golded3/gelmsg.cpp:134
#10 0x000000000046fd9b in FindString (msg=0xd1ebec, prompt=<optimized out>,
what=<optimized out>) at /home/fido/src/golded-plus/golded3/gmarea.h:1214
#11 0x00000000004701d1 in FindAll (msg=***@entry=0xd1ebec, topline=@0x7ff8f4:
0, keyok=@0x7ff8f0: 0) at /home/fido/src/golded-plus/golded3/gmarea.h:985
#12 0x00000000004a7520 in Reader () at
/home/fido/src/golded-plus/golded3/geread.cpp:872
#13 0x000000000040999e in main (argc=<optimized out>, argv=<optimized out>) at
/home/fido/src/golded-plus/golded3/gemain.cpp:54

Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю, разбираться
ночью с этим багом я не буду, не on-call я сегодня по голдеду.

Best Regards, Nil

[Vitaliy Aksyonov]

Привет, Nil!

11 Aug 23 03:22, ты писал(а) All:

NA> Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю,
NA> разбираться ночью с этим багом я не буду, не on-call я сегодня по
NA> голдеду.

Так неспортивно. А Steps to reproduce? Ну или корку на крайняк.

Best regards,
Vitaliy Aksyonov.

... Вот такие загадочные эти зверьки, female'сы.

[Nil A]

Hello, Vitaliy!

Friday August 11 2023 12:02, from Vitaliy Aksyonov -> Nil A:

NA>> Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю,
NA>> разбираться ночью с этим багом я не буду, не on-call я сегодня по
NA>> голдеду.
VA> Так неспортивно. А Steps to reproduce? Ну или корку на крайняк.

В hobbit.test эхе хулиганят, написали вот такой вот длинный ориджин

* Origin: И стали драться Илья Муромец со змеем Горынычем и отрубил
Муромец одну голову Змею. Но выросло у того 2. И отрубил он у змея 2
головы, но выросло 4... и отрубил он у Горыныча 65535

и фсё, голдед на свою голую попу сел. Причём, там есть нюанс, когда билд -O0 и
-O1,.. может в кору упасть, а может и не упасть, точнее в другом месте упадёт
;-)

Так то строчка выглядит норм

strxcpy(msg->origin, line->txt.c_str()+11, sizeof(msg->origin));

Чертовски длинный ориджин лежит в стринге line->txt, msg->origin это char[160]
просто. Я уж не буду комментировать +11 отступ, чтобы пропустить " * Origin: "
;-)
Внутри strxcpy() делает strcpy() размер -1, и закрывает \0 буфер - всё вроде
хорошо.



Короче, вот что я точно знаю, что Одинн Соренсен (царство ему небесное), он был
кулхацкером в 90е года, когда плюсы были "Си с классами". Его хакерский стиль -
это переиспользовать структурки, просто memset() затираем все поля, и дальше
едем. Конструкторы/декструкторы - это для слабаков, мы сейчас сэкономим на
new/delete. Окей, всё будет работать, пока у тебя структурка просто POD (типа
std::is_trivial_v), но когда там добавили стринги..

А потом пришли чувачки, невкурили они идею Одинна, и втащили местами
std::string с std::vector, и структурки стали уже не POD. Я как-то под valgrind
запустил, и там потерянная память, потому что кулхацкер Одинн memset() затирает
структурки, а стринги и вектора никто не деструктирует как положено. Короче,
там где стринги и вектора, то там новодел - инфа 100%.



Да говно код вашего голдеда, вот чесно. Думал, сейчас голыми руками голдеда
поймаю, собрал АСАН билд

-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all
-fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow
-fno-sanitize=null -fno-sanitize=alignment

и что? До списка эх не дошёл, уже арифметика указателей нитаво

goldlib/gall/gmemdbg.cpp:130:53: runtime error: pointer index expression
with base 0x000000000000 overflowed to 0xffffffffffffffd4

Вот тут хитро прячут структурку Throw за адресом буфера - тот ещё хак. Ну блин,
не свой же кастомный маллок пишем.

(Throw*)((const char*)ptr-sizeof(Throw)+1);

Ну окей, мне сейчас надо чисто санитайзер на адреса сделать, просто один
-fsanitize=address. Опять до списка эх не дошёл.

AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs operator
delete)

Да блин, там десятилетиями эти баги сидят, и всем пофиг. Надо прораваться
дальше, запускаем с ASAN_OPTIONS=alloc_dealloc_mismatch=0 -у нас список эх уже,
нажимаем клавишу названия эхи, и...

AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffded765a05
at pc 0x00000071d98d bp 0x7ffded765910 sp 0x7ffded765908

Стрёмненько как-то вобщем.

Best Regards, Nil

[Nil A]

Hello, Vitaliy!

Friday August 11 2023 12:02, from Vitaliy Aksyonov -> Nil A:

NA>> Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю,
NA>> разбираться ночью с этим багом я не буду, не on-call я сегодня по
NA>> голдеду.
VA> Так неспортивно. А Steps to reproduce? Ну или корку на крайняк.

Ларчик легко открывался.
TLDR; надо бы все strcat() в коде на strncat() заменить, а иначе классический
buffer overflow.

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

Имеется - хорошее воспроизведение бага. При поиске "Enter Searchstring
(Header+Text)", по hobbit.test, у меня shift+f, или option+f кнопка, вводим
пофиг что. Как оказалось, не пофиг размер окна терминала, т.е. $COLUMNS у меня
сейчас 148, если совсем маленький, или совсем широкий, то не воспроизводится
баг.

Начинаем ковырять, собираем из гитхаб мастера с -O0 -g. Напускаем gdb, видим,
что в разных функциях, где передаётся *Gmg msg, и потом, когда достают
msg->lin, то там оказывается всегда одинаковое фиксированное значение, и это
явно за пределами памяти.

(gdb) up 2
#2 0x00000000004852e9 in MsgLineReIndex (msg=0xd893cc, viewhidden=0,
viewkludge=0, viewquote=1)
at /home/fido/src/golded-plus/golded3/geline.cpp:3111
3111 throw_xrelease(msg->line);
(gdb) p msg->lin
$1 = (Line *) 0xd9ced9d2cfe720d5
(gdb) p *msg->lin
Cannot access memory at address 0xd9ced9d2cfe720d5

Сначала я в gdb пытался watch разные ставить, чтобы отловить, кто туда пишет.
Оказалось, что мой линукс в виртуалке, которая на OpenVZ, ещё и плохо с
хардварными вочпоинтами дружит, типа не пермишен.

Потом я думал, что отловлю на ASAN билде, кто в какую память пишет не так. Но
там всё совсем плохо оказалось, я уже писал ранее. Хотя, сейчас понимаю, что
рано сдался, надо было с ASAN_OPTIONS=halt_on_error=0 пускать, и он бы прошагал
дальше.

Потом я запустил под valgrind, который, сначала обругался, что у нас в if
выражениях используются значения неинициализованных переменных, в районе
Initialize->ReadKeysCfg. Потом ругань на несоответствия free() / delete /
delete [], которые я уже видел на ASAN билде. Кстати, может кто-нибудь это всё
вычистит?

И вот тут звёзды сошлись. Valgrind сказал, что пишем за пределы вот тут
by 0x48156C: ScanKludges(GMsg*, int) (geline.cpp:1743)
Там вот такая строчка
strcat(msg->origin, line->next->txt.c_str());

И, как и ожидалось, в структурке GMsg, за "char origin[160]" (который оказался
чуть-чуть длинноватым в сообщении), следующее поле "Line* lin". Таким образом
этот указатель съезжает на левый адрес, а дальше разыменование указателя это
UB.

Best Regards, Nil

[Nil A]

Hello, All!

Friday August 11 2023 03:22, from Nil A -> All:

NA> Я имею устойчивый репродакшен баг, на голдеде, построенном из гитхаб
NA> мастер.

Вот такой простой фикс чинит проблему, с которой я сталкнулся, когда перцы
делают Ориджин длиннее 160 символов.

-+- a/golded3/geline.cpp
+++ b/golded3/geline.cpp
@@ -1740,7 +1740,7 @@ void ScanKludges(GMsg* msg, int getvalue)
strxcpy(msg->origin, line->txt.c_str()+11,
sizeof(msg->origin));
if(nextor) // Get the next line too
{
- strcat(msg->origin, line->next->txt.c_str());
+ strxcat(msg->origin, line->next->txt.c_str(),
sizeof(msg->origin));
line->next->color = C_READO;
line->next->type |= GLINE_ORIG; // Mark next
line as Origin too
}

P.S. Бонусом расскажу прикол заодно. Здесь sizeof(msg->origin) можно написать
без скобок, потому sizeof не функция, а оператор. Если sizeof с именем
переменной, то скобки можно не ставить, а если с типом надо писать. Казалось
бы, кого это волнует? А вот, у меня на работе такие умники есть, на код-ревью
мне такие комментарии пишут, просто в остальном сложно по коду докопаться. :-)


NA> Program received signal SIGABRT, Aborted.
NA> gdb) bt
NA> #0 0x00007ffff7099c37 in __GI_raise (sig=***@entry=6) at
NA> ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007ffff709d028 in
NA> __GI_abort () at abort.c:89 #2 0x00007ffff70d62a4 in __libc_message
NA> (do_abort=***@entry=2, fmt=***@entry=0x7ffff71e5db0 "*** %s ***:
NA> %s terminated\n") at ../sysdeps/posix/libc_fatal.c:175 #3
NA> 0x00007ffff717187c in __GI___fortify_fail (msg=<optimized out>,
NA> ***@entry=0x7ffff71e5d47 "buffer overflow detected") at
NA> fortify_fail.c:38 #4 0x00007ffff7170750 in __GI___chk_fail () at
NA> chk_fail.c:28 #5 0x00007ffff716f8eb in __strcat_chk (
NA> dest=***@entry=0xd20488 "\351 \323\324\301\314\311
NA> \304\322\301\324\330\323\321 \351\314\330\321
NA> \355\325\322\317\315\305\303 \323\317 \332\315\305\305\315
NA> \347\317\322\331\316\331\336\305\315 \311 \317\324\322\325\302\311\314
NA> \355\325\322\317\315\305\303 \317\304\316\325 \307\317\314\317\327\325
NA> \372\315\305\300. \356\317 \327\331\322\317\323\314\317 \3257 #6
NA> 0x000000000047dad6 in strcat (__src=<optimized out>,
NA> __dest=0xd20488 "\351 \323\324\301\314\311
NA> \304\322\301\324\330\323\321 \351\314\330\321
NA> \355\325\322\317\315\305\303 \323\317 \332\315\305\305\315
NA> \347\317\322\331\316\331\336\305\315 \311 \317\324\322\325\302\311\314
NA> \355\325\322\317\315\305\303 \317\304\316\325 \307\317\314\317\327\325
NA> \372\315\305\300. \356\317 \327\331\322\317\323\314\317 \325 \324\3172
NA> #7 ScanKludges (msg=***@entry=0xd1ebec, getvalue=***@entry=1) at
NA> /home/fido/src/golded-plus/golded3/geline.cpp:1743 #8
NA> 0x000000000047fe80 in MakeLineIndex (msg=0xd1ebec, margin=141,
NA> getvalue=true, header_recode=<optimized out>) at
NA> /home/fido/src/golded-plus/golded3/geline.cpp:3071 #9
NA> 0x00000000004817e9 in Area::LoadMsg (this=***@entry=0xbab570,
NA> msg=***@entry=0xd1ebec, msgno=8388, margin=***@entry=141,
NA> mode=***@entry=0) at
NA> /home/fido/src/golded-plus/golded3/gelmsg.cpp:134 #10
NA> 0x000000000046fd9b in FindString (msg=0xd1ebec, prompt=<optimized
NA> out>, what=<optimized out>) at
NA> /home/fido/src/golded-plus/golded3/gmarea.h:1214 #11
NA> 0x00000000004701d1 in FindAll (msg=***@entry=0xd1ebec,
NA> topline=@0x7ff8f4: 0, keyok=@0x7ff8f0: 0) at
NA> /home/fido/src/golded-plus/golded3/gmarea.h:985 #12 0x00000000004a7520
NA> in Reader () at /home/fido/src/golded-plus/golded3/geread.cpp:872 #13
NA> 0x000000000040999e in main (argc=<optimized out>, argv=<optimized
NA> out>) at /home/fido/src/golded-plus/golded3/gemain.cpp:54

Best Regards, Nil

[Sergey Myasoedov]

Hi Nil!

13 Aug 23 07:26, you wrote to All:

NA> Вот такой простой фикс чинит проблему, с которой я сталкнулся, когда перцы
NA> делают Ориджин длиннее 160 символов.

Чего ж ты коммит не залил?

[Nil A]

Hello, Sergey!

Tuesday August 22 2023 22:04, from Sergey Myasoedov -> Nil A:

NA>> Вот такой простой фикс чинит проблему, с которой я сталкнулся,
NA>> когда перцы делают Ориджин длиннее 160 символов.
SM> Чего ж ты коммит не залил?

Вкуда комиит залил? Мне чё щас ещё на гитхабе, может быть, зарегистрироваться
(или где его там хостят)?
Ага, потом ещё в телегремме зарегаться, потом ещё во вконтекте, потом ещё
воцапе, потом ещё фейсбуки, тиндеры-шминдеры...

Фидо самодостаточное, или таковым должно быть, ибо сеть целая!
Я, кстати, прям готов даже кодревью делать, вот прям вот так вот, используя
эхомейл, с минимальным форматированием. Не Маркдаун, конечно, но _могу_ вот
*так*, и вообще по

всякому могу

Между прочим. Все линуксо-кернеловские патчи летали всегда просто в мейл-листе,
там же и на них комменты писали. А тут, видишь ли, фидоНЕТ не самодостаточен,
оказывается!

Best Regards, Nil