Bugs in GoldEd

Nil Alexandrov написал(а) к All в May 21 06:07:18 по местному времени:

Нello, All!

Правильно я понимаю, что последняя версия golded лежит https://github.com/golded-plus/golded-plus ?

Далее следуют мои наблюдения.

1. char buf[256], options[80];
Много функций имеют такие вот фиксированные буфера на стеке для работы с 80x25 терминалом, если запускать в Linux/Mac широком терминале, то происходит переполнение.
Быстрое лечение - замена на бОльшие значения буферов.
Правильное лечение - узнать размер терминала, выделить буфер соответственно, ещё надо ловить сигналы, когда размер терминала меняется и перевыделять буфера и перерисовывать экран по-хорошему - это много кода и он платформенно зависимый.

2. Overflow
Прочие переполнения фиксированных буферов, например, когда идёт цветовое выделение фрагмента, можно предположить худший сценарий, когда весь текст будет выделен.
-+- a/golded3/gectnr.cpp
+++ b/golded3/gectnr.cpp
@@ -95,7 +95,7 @@ void Container::StyleCodeНighlight(const char* text, int row, int col, bool dohi

uint sclen = 0;
const char* txptr = text;
- char buf[200];
+ char buf = (char)throw_calloc(1, strlen(text)+1);
const char* ptr = text;
const char* stylemargins = " -|\\"; // we probably have to make a keyword for it
char* punctchars = CFG->stylecodepunct;

3. New delete type mismatch
Есть места, где буфер выделают под массив, а освобождают как переменную, например
-+- a/golded3/gearea.cpp
+++ b/golded3/gearea.cpp
@@ -151,8 +151,8 @@ void GPickArealist::do_delayed()
LOG.printf("! Buffer overflow: tmp in GPickArealist::do_delayed(), 15 or 16 lines above");
PointerErrorExit();
}
- delete buf;
- delete tmp;
+ delete[] buf;
+ delete[] tmp;
}


-+- a/golded3/geinit.cpp
+++ b/golded3/geinit.cpp
@@ -536,7 +536,7 @@ void Uninitialize()
{
if (WideUsername)
{
- delete WideUsername;
+ delete[] WideUsername;
WideUsername = NULL;
}
LookupNodeClear();

4. Strcpy param overlap
Есть код, где нужно удалить часть строки, например начальные Re: - это реализуется путём копирования подстроки. Во многих местах для этого используется strcpy(), тогда как для перекрывающихся участков памяти нужно использовать memmove.
-+- a/golded3/gepost.cpp
+++ b/golded3/gepost.cpp
@@ -1076,7 +1076,7 @@ void MakeMsg(int mode, GMsg* omsg, bool ignore_replyto)
ptr = strskip_wht(r);
}
while(r);
- strcpy(msg->re, ptr);
+ memmove(msg->re, ptr, strlen(ptr)+1);
}
strcpy(msg->ddom, omsg->odom);
if((mode == MODE_REPLYCOMMENT) and omsg->fwdorig.net)
-+- a/golded3/getpls.cpp
+++ b/golded3/getpls.cpp
@@ -350,17 +350,17 @@ int TemplateToText(int mode, GMsg msg, GMsg* oldmsg, const char tpl, int origa

// build @dpseudo
if(msg->to_me())
- strcpy(oldmsg->pseudoto, msg->pseudofrom);
+ memmove(oldmsg->pseudoto, msg->pseudofrom, strlen(msg->pseudofrom)+1);
else if(msg->to_you())
- strcpy(oldmsg->pseudoto, msg->pseudoto);
+ memmove(oldmsg->pseudoto, msg->pseudoto, strlen(msg->pseudoto)+1);
else
*(oldmsg->pseudoto) = NUL;

// build @opseudo
if(msg->by_me())
- strcpy(oldmsg->pseudofrom, msg->pseudofrom);
+ memmove(oldmsg->pseudofrom, msg->pseudofrom, strlen(msg->pseudofrom)+1);
else if(msg->by_you())
- strcpy(oldmsg->pseudofrom, msg->pseudoto);
+ memmove(oldmsg->pseudofrom, msg->pseudoto, strlen(msg->pseudoto)+1);
else
*(oldmsg->pseudofrom) = NUL;

5. Memset() clearing an object with no trivial copy-assignment.
С многими C++ объектами работают, как если бы это были C-структуры с POD типами, например, инициализация объекта сообщений происходит так memset(msg, 0, sizeof(GMsg)), хотя это наследуемый класс от gmsg, и там есть много сложных объектов типа ftnaddr, или даже gmsg_links, внутри которого есть std::vector. Также, с целью оптимизации выделения/освобождения памяти, переиспользуют такие объекты, просто забивая их нулями. Благодаря счастливому стечению обстоятельств, нули работают как и для char*, и для ftnaddr и даже, скорее всего, для локальной реализации std::vector, который хранит указатель на буфер и размер. Что точно ломается - это память, которую выделяет внутри std::vector.
По хорошему, надо сделать рефакторинг, чтобы инициализировать объекты C++ соответствующими для этого методами.
Быстрый хак, чтобы не утекала память под std::vector, работает только начиная с C++11.
-+- a/goldlib/gmb3/gmo_msg.h
+++ b/goldlib/gmb3/gmo_msg.h
@@ -250,6 +250,7 @@ public:
{
replyto = reply_first = replynext = 0;
reply_list.clear();
+ replylist.shrink_tofit();
}

int list_max() const

6. Memory leaks
Наверное их много, сходу ловиться вот такой.
-+- a/goldlib/gall/gespell.cpp
+++ b/goldlib/gall/gespell.cpp
@@ -834,6 +834,9 @@ void CSpellChecker::Close()
{
if (!IsInited()) return;
UnLoad();
+ CSpellLangV::iterator it;
+ for (it = mLangs.begin(); it != mLangs.end(); it++)
+ delete *it;
mLangs.clear();
mInited = false;
}

7. ISO C++ forbids converting a string constant to char*
Все строковые константы из .text сегмента передаются дальше по коду без константы, что провоцирует много предупреждений со стороны компилятора. Хочется надеяться, что ни какой код не начёт переписать содержимое таких буферов.

Best Regards, Nil
--- GoldED+/LNX 1.1.5

Eugene Palenock написал(а) к Nil Alexandrov в May 21 11:50:52 по местному времени:

Привет, Nil!

09 май 21 06:07, Nil Alexandrov -> All:

NA> Далее следуют мои наблюдения.

Напомню ещё один баг - повреждение текста при стилях выделения в квотинге.

Без кавычек эти три строки GoldEd не сохраняет:
"ZZZZ<YYYY"
"X)>AAA"
"X)>BBB"

Они превращаются в такое при сохранении:
"ZZZZ<YYYY"
"X)>AAA BBB"

Коллизия случается при совокупности условий:
- подряд три этих строки где-либо в текcте;
- в 1 строке есть стрелка влево в любом месте строки;
- в начале 2 и 3 строки нечто похожее на квотинг;
- причём 2 буква должна быть не буквой а каким-нибудь символом (если там "XX>"
то коллизии нет, а например "X=>" или "X(>" вызывает коллизию).


--
С уважением, Евгений.

---

Semen Panevin написал(а) к Nil Alexandrov в May 21 17:26:18 по местному времени:

Доброго здоровьица тебе, Nil!

Sunday May 09 2021 06:07, Nil Alexandrov писал All:

NA> Далее следуют мои наблюдения.
Если ты ещё найдёшь отчего у меня голдед при включенной проверке орфографии крашится, а у других не крашится (и у меня раньше не крашился, а что изменилось - хрен его знает) - будет вообще супер...

С наилучшими пожеланиями, Семён.

... Век живи, век лечись...
--- GoldED+/LNX 1.1.5-b20180707 (Linux 5.4.80-gentoo-r1 iF6M10)

Alexander Polozov написал(а) к Semen Panevin в May 21 23:03:52 по местному времени:

Привет, Semen!


Ответ на сообщение Semen Panevin (2:5025/121) к Nil Alexandrov, написанное 09 май 21 в 17:26:

NA>> Далее следуют мои наблюдения.
SP> Если ты ещё найдёшь отчего у меня голдед при включенной проверке
SP> орфографии крашится, а у других не крашится (и у меня раньше не
SP> крашился, а что изменилось - хрен его знает) - будет вообще супер...
Ну найти это вряд-ли, глюк старый, известный и до сих пор ни кем не отловленный...
Зато довольно давно известно лечение, сохраняешь старый конфиг в бекап, создаёшь новый пустой конфиг, в который потом по одной строке переносишь только необходимое (у меня на выходе новый конфиг уложился в 96 строк, включая пустые для красоты), на малом количестве строк в конфиге глюк со спелчеком не вылазит.

Со всеми пожеланиями, Alexander Polozov

... Шли к рынку, а пришли к базару.
--- GoldED+/LNX 1.1.5 (Linux 5.10.27-gentoo CPU UNKNOWN)

Mykhailo Kapitanov написал(а) к Nil Alexandrov в May 21 21:38:08 по местному времени:


Нello Nil!

09 May 21 06:07, you wrote to all:

NA> 1. char buf[256], options[80];
NA> Много функций имеют такие вот фиксированные буфера на стеке для работы
NA> с 80x25 терминалом, если запускать в Linux/Mac широком терминале, то

В версии для Windows та же проблема.

Mykhailo


--- GoldED+/W64-MSVC 1.1.5-b20180707

Nil A написал(а) к Vitaliy Aksyonov в Oct 23 20:27:42 по местному времени:

Нello, Vitaliy!

Тебе ещё один забавный баг в копилку.

===============================================================================
From : Eugene Palenock 2:5020/2065 Sun 09 May 21 11:50
To : Nil Alexandrov
Subj : Bugs in GoldEd
===============================================================================
Привет, Nil!

09 май 21 06:07, Nil Alexandrov -> All:

NA> Далее следуют мои наблюдения.

Напомню ещё один баг - повреждение текста при стилях выделения в квотинге.

Без кавычек эти три строки GoldEd не сохраняет:
"ZZZZ<YYYY"
"X)>AAA"
"X)>BBB"

Они превращаются в такое при сохранении:
"ZZZZ<YYYY"
"X)>AAA BBB"

Коллизия случается при совокупности условий:
- подряд три этих строки где-либо в текcте;
- в 1 строке есть стрелка влево в любом месте строки;
- в начале 2 и 3 строки нечто похожее на квотинг;
- причём 2 буква должна быть не буквой а каким-нибудь символом (если там "XX>"
то коллизии нет, а например "X=>" или "X(>" вызывает коллизию).


--
С уважением, Евгений.

-+-