Используется memcpy() для строк, не добавляется '\0'
ekzolol opened this issue · 2 comments
Статический анализатор PVS Studio выдал такие ошибки:
contrib/pg_query_state/signal_handler.c 125 err V575 The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null.
contrib/pg_query_state/signal_handler.c 129 err V575 The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null.
Вот, собственно, тот кусок, на который он ругается:
static void
serialize_stack_frame(char **dest, stack_frame *qs_frame)
{
SET_VARSIZE(*dest, strlen(qs_frame->query) + VARHDRSZ);
memcpy(VARDATA(*dest), qs_frame->query, strlen(qs_frame->query));
*dest += INTALIGN(VARSIZE(*dest));
SET_VARSIZE(*dest, strlen(qs_frame->plan) + VARHDRSZ);
memcpy(VARDATA(*dest), qs_frame->plan, strlen(qs_frame->plan));
*dest += INTALIGN(VARSIZE(*dest));
}
Я, если честно, так сразу не понимаю, false positive это или нет. Зависит от контекста.
Например, если данные из dest где-то печатаются, то в конце должен быть '\0', иначе это undefined behaviour. И если под dest изначально выделен фиксированный участок памяти, а то, что мы туда копируем, может быть неограниченно длинным, то неплохо было бы проверять длину и убеждаться, что мы не выйдем за этот фиксированный участок памяти.
Привет, @ekzolol
PVS Studio - это хорошо, наверняка предупреждений было еще больше. Возможно, вы нашли какой-то баг и решили проверить код?
Я не являюсь автором этого расширения, но беглый просмотр кода свидетельствует не в пользу ваших замечаний:
- Во-первых, длина заведомо известна, и ее вычисляет функция serialized_stack_length().
- Во-вторых, перед каждой строкой пишется ее длина (SET_VARSIZE()).
Привет @ekzolol ! Спасибо за проверку исходного кода расширения анализатором PVS Studio.
Как написал @funbringer , '\0' символ на конце не нужен, фактическая длина строки сохраняется перед символьным массивом функцией SET_VARSIZE(). Строка запроса и дерево плана хранятся в виде встроенного типа text, который определён следующим образом:
typedef struct varlena text;
struct varlena
{
char vl_len_[4]; /* Do not touch this field directly! */
char vl_dat[FLEXIBLE_ARRAY_MEMBER]; /* Data content is here */
};
Внутри расширения эти строки нигде не печатаются, они возвращаются в качестве результата pg_query_state().
Что касается аллокации памяти под последовательность пар <тело запроса, план запроса>, то весь размер необходимой памяти изначально вычисляется через функцию serialized_stack_length() и динамически выделяется во фрагменте функции SendQueryState():
int msglen = sizeof(shm_mq_msg) + serialized_stack_length(qs_stack);
shm_mq_msg *msg = palloc(msglen);
На мой взгляд, PVS Studio сработал из-за наличия комбинации вызовов memcpy/strlen функций, предположив, что копируются строки в их классическом представлении.