Detect buffer underflow in get_th()

Started by Alexander Kuznetsovover 1 year ago6 messageshackers
Jump to latest
#1Alexander Kuznetsov
kuznetsovam@altlinux.org

Hello everyone,

In src/backend/utils/adt/formatting.c:1516, there is a get_th() function utilized to return ST/ND/RD/TH suffixes for simple numbers.
Upon reviewing its behavior, it appears capable of receiving non-numeric inputs (this is verified by a check at formatting.c:1527).

Given that the function can accept non-numeric inputs,
it is plausible that it could also receive an empty input,
although a brief examination of its calls did not reveal any such instances.

Nevertheless, if the function were to receive an empty input of zero length,
a buffer underflow would occur when attempting to compute *(num + (len - 1)), as (len - 1) would result in a negative shift.
To mitigate this issue, I propose a patch incorporating the zero_length_character_string error code, as detailed in the attachment.

--
Best regards,
Alexander Kuznetsov

Attachments:

0001-Detect-buffer-underflow-in-get_th.patchtext/x-patch; charset=UTF-8; name=0001-Detect-buffer-underflow-in-get_th.patchDownload+5-1
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Alexander Kuznetsov (#1)
Re: Detect buffer underflow in get_th()

On 24.07.24 11:43, Alexander Kuznetsov wrote:

Hello everyone,

In src/backend/utils/adt/formatting.c:1516, there is a get_th() function
utilized to return ST/ND/RD/TH suffixes for simple numbers.
Upon reviewing its behavior, it appears capable of receiving non-numeric
inputs (this is verified by a check at formatting.c:1527).

Given that the function can accept non-numeric inputs,
it is plausible that it could also receive an empty input,
although a brief examination of its calls did not reveal any such
instances.

Nevertheless, if the function were to receive an empty input of zero
length,
a buffer underflow would occur when attempting to compute *(num + (len -
1)), as (len - 1) would result in a negative shift.
To mitigate this issue, I propose a patch incorporating the
zero_length_character_string error code, as detailed in the attachment.

If it can't happen in practice, maybe an assertion would be enough?

#3Alexander Kuznetsov
kuznetsovam@altlinux.org
In reply to: Peter Eisentraut (#2)
Re: Detect buffer underflow in get_th()

24.07.2024 18:39, Peter Eisentraut wrote:

If it can't happen in practice, maybe an assertion would be enough?

In practice, the function should not receive non-numeric strings either; however, since there is an exception in place for such cases, I thought it would be good to add a check for zero-length input in a similar manner.

But of course it's open for discussion and team decision whether this should be addressed as an assertion or handled differently.

--
Best regards,
Alexander Kuznetsov

#4Alexander Kuznetsov
kuznetsovam@altlinux.org
In reply to: Alexander Kuznetsov (#3)
Re: Detect buffer underflow in get_th()

Hello,

is there anything else we can help with or discuss in order to apply this fix?

24.07.2024 18:53, Alexander Kuznetsov пишет:

24.07.2024 18:39, Peter Eisentraut wrote:

If it can't happen in practice, maybe an assertion would be enough?

In practice, the function should not receive non-numeric strings either; however, since there is an exception in place for such cases, I thought it would be good to add a check for zero-length input in a similar manner.

But of course it's open for discussion and team decision whether this should be addressed as an assertion or handled differently.

--
Best regards,
Alexander Kuznetsov

#5Alexander Kuznetsov
kuznetsovam@altlinux.org
In reply to: Alexander Kuznetsov (#4)
Re: Detect buffer underflow in get_th()

Hello, ping?

24.09.2024 17:52, Alexander Kuznetsov wrote:

Hello,

is there anything else we can help with or discuss in order to apply this fix?

24.07.2024 18:53, Alexander Kuznetsov пишет:

24.07.2024 18:39, Peter Eisentraut wrote:

If it can't happen in practice, maybe an assertion would be enough?

In practice, the function should not receive non-numeric strings either; however, since there is an exception in place for such cases, I thought it would be good to add a check for zero-length input in a similar manner.

But of course it's open for discussion and team decision whether this should be addressed as an assertion or handled differently.

--
Best regards,
Alexander Kuznetsov

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Alexander Kuznetsov (#5)
Re: Detect buffer underflow in get_th()

On 13.12.24 09:57, Alexander Kuznetsov wrote:

Hello, ping?

24.09.2024 17:52, Alexander Kuznetsov wrote:

Hello,

is there anything else we can help with or discuss in order to apply
this fix?

24.07.2024 18:53, Alexander Kuznetsov пишет:

24.07.2024 18:39, Peter Eisentraut wrote:

If it can't happen in practice, maybe an assertion would be enough?

In practice, the function should not receive non-numeric strings
either; however, since there is an exception in place for such cases,
I thought it would be good to add a check for zero-length input in a
similar manner.

But of course it's open for discussion and team decision whether this
should be addressed as an assertion or handled differently.

After some further (internal) consultations, I have committed your fix
as an assertion. Thanks.