[HACKERS] Code quality issues in ICU patch

Поиск
Список
Период
Сортировка
От Tom Lane
Тема [HACKERS] Code quality issues in ICU patch
Дата
Msg-id 23278.1498235500@sss.pgh.pa.us
обсуждение исходный текст
Ответы Re: [HACKERS] Code quality issues in ICU patch  (David Fetter <david@fetter.org>)
Re: [HACKERS] Code quality issues in ICU patch  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Список pgsql-hackers
icu_to_uchar() and icu_from_uchar(), and perhaps other places, are
touchingly naive about integer overflow hazards in buffer size
calculations.  I call particular attention to this bit in
icu_from_uchar():
len_result = UCNV_GET_MAX_BYTES_FOR_STRING(len_uchar, ucnv_getMaxCharSize(icu_converter));

The ICU man pages say that that macro is defined as

#define UCNV_GET_MAX_BYTES_FOR_STRING(length, maxCharSize)     (((int32_t)(length)+10)*(int32_t)(maxCharSize))

which means that getting this to overflow (resulting in
probably-exploitable memory overruns) would be about as hard as taking
candy from a baby.

I also notice that the general approach to handling ICU-reported
error conditions is like
   if (U_FAILURE(status))       ereport(ERROR,               (errmsg("ucnv_fromUChars failed: %s",
u_errorName(status))));

This lacks an errcode() setting, which is contrary to project policy,
and the error message violates our message style guidelines.

I don't particularly feel like fixing these things myself, but
somebody needs to; the overflow issues in particular are stop-ship
security hazards.
        regards, tom lane



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Sergey Burladyan
Дата:
Сообщение: Re: [HACKERS] Broken hint bits (freeze)
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: [HACKERS] Setting pd_lower in GIN metapage