Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Сборка для FreeBSD и много предупреждений #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tarkhil
Copy link

@tarkhil tarkhil commented Dec 18, 2020

Заменены везде, где нашел, неправильные типы.

В XXI веке лучше использовать size_t, чем uint32

@@ -371,12 +372,12 @@ RESPONSECODE CmdXfrBlock(unsigned int reader_index, unsigned int tx_length,
iscase4 = 1; /* FIXME: */
case IFD_APDU_CASE_1:
r = CmdSendTPDU(reader_index, send_buf, tx_length,
rx_buffer, *rx_length, &rrecv, iscase4);
rx_buffer, *rx_length, (size_t *)&rrecv, iscase4);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Каст int* к size_t* выглядит как попытка внести уязвимость.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну, по уму, там нужно всю функцию переписать от слова совсем. С заменой разработчика, извините.

Каст int к size_t выглядит как я даже не знаю что.

CmdTranslateRxBuffer ожидает на входе int rrecv, но данные в этот rrecv заносятся как в size_t. У меня, честно, не хватило сил лезть до самого уровня системных вызовов, но 99% - это все-таки size_t, которому пытаются присвоить -1 как признак "ничего мы не получали", а проверяется ли это где-то в цепочке вызово - я понять не осилил.

Ну, то есть, я ставил целью - чтоб собралось без ошибок и предупреждений и работало. Пытаться что-то сломать путем записи токена с какой-нибудь странной длиной я даже не думал. Хотя мысль интересная.

@@ -52,41 +52,41 @@ extern int LogLevel;
#include <debuglog.h>

/* DEBUG_CRITICAL */
#define DEBUG_CRITICAL(fmt) if (LogLevel & DEBUG_LEVEL_CRITICAL) Log1(PCSC_LOG_CRITICAL, fmt); else (fmt)
#define DEBUG_CRITICAL(fmt) if (LogLevel & DEBUG_LEVEL_CRITICAL) { Log1(PCSC_LOG_CRITICAL, fmt); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, понял, сайд-эффекты отладки. Тут, наверное, лучше оставить, как было

@Mironenko
Copy link
Contributor

@tarkhil, спасибо за PR. Много warning-ов -- естественно, плохо. Постараемся посмотреть, что можно с этим сделать.

@tarkhil
Copy link
Author

tarkhil commented Dec 19, 2020

Там в каком-то месте просто не собиралось, нужный include не подключался. Ну, и, чтоб два раза не вставать, я все предупреждения с типами исправил. Одно - да, очень кувалдой.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants