Thread: 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 simplenumbers. 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
Attachment
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 forsuch 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 placefor 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 placefor 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