Detect buffer underflow in get_th()
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
From de36780802b72b643b47ec129979292e9832e9e7 Mon Sep 17 00:00:00 2001
From: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Date: Wed, 24 Jul 2024 12:31:45 +0300
Subject: [PATCH] Detect buffer underflow in get_th()
If get_th() can receive input that is not a number,
then it can also receive empty input.
Empty input with zero length can result in a buffer underflow
when accessing *(num + (len - 1)), as (len - 1) would produce a negative index.
Add a check for zero-length input to prevent it.
This was found by ALT Linux Team.
---
src/backend/utils/adt/formatting.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 8736ada4be..0ab5ac5bf0 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -1518,6 +1518,11 @@ get_th(char *num, int type)
int len = strlen(num),
last;
+ if (len == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_ZERO_LENGTH_CHARACTER_STRING),
+ errmsg("input cannot be empty string")));
+
last = *(num + (len - 1));
if (!isdigit((unsigned char) last))
ereport(ERROR,
--
2.42.2
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?
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
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
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
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.