BUG #18614: [ECPG] out of bound in DecodeDateTime

On Thu, Sep 12, 2024 at 08:54:42AM +0000, PG Bug reporting form wrote:

The following bug has been logged on the website:

Bug reference: 18614
Logged by: Pavel Nekrasov
Email address: p.nekrasov@fobos-nt.ru
PostgreSQL version: 17rc1
Operating system: Alt 10
Description:

in the line ```if (tm->tm_mday < 1 || tm->tm_mday >
day_tab[isleap(tm->tm_year)][tm->tm_mon - 1]) ``` tm->tm_mon may be equal to
0, which will result in reading by indexes -1

this is possible when calling PGTYPESdate_from_asc or
PGTYPEStimestamp_from_asc with "str" equal, for example, "AM95000062"

Yes, I agree. Attached is a non-word-wrapped patch that I plan to apply
to all supported PG versions.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

On Mon, Oct 14, 2024 at 05:52:59PM -0400, Bruce Momjian wrote:

/*
-		 * check for valid day of month, now that we know for sure the month
+		 * check for valid day of month and month, now that we know for sure the month
* and year...
*/
-		if (tm->tm_mday < 1 || tm->tm_mday > day_tab[isleap(tm->tm_year)][tm->tm_mon - 1])
+		if (tm->tm_mon < 1 || tm->tm_mday < 1 || tm->tm_mday > day_tab[isleap(tm->tm_year)][tm->tm_mon - 1])
return -1;

I would suggest adding a test. This is tricky enough and really easy
to miss, even if ECPG is not that touched these days.

There are a lot of callers of PGTYPEStimestamp_from_asc() in
dt_test.pgc that do sanity checks on such inputs.
--
Michael

On Wed, Oct 16, 2024 at 09:18:10AM +0900, Michael Paquier wrote:

On Mon, Oct 14, 2024 at 05:52:59PM -0400, Bruce Momjian wrote:

/*
-		 * check for valid day of month, now that we know for sure the month
+		 * check for valid day of month and month, now that we know for sure the month
* and year...
*/
-		if (tm->tm_mday < 1 || tm->tm_mday > day_tab[isleap(tm->tm_year)][tm->tm_mon - 1])
+		if (tm->tm_mon < 1 || tm->tm_mday < 1 || tm->tm_mday > day_tab[isleap(tm->tm_year)][tm->tm_mon - 1])
return -1;

I would suggest adding a test. This is tricky enough and really easy
to miss, even if ECPG is not that touched these days.

There are a lot of callers of PGTYPEStimestamp_from_asc() in
dt_test.pgc that do sanity checks on such inputs.

Added in the attached patch. Is the output correct?

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

On Thu, Oct 17, 2024 at 01:29:03PM -0400, Bruce Momjian wrote:

Added in the attached patch. Is the output correct?

Well, no. The result in the regression test output you are producing
is a consequence that PGTYPEStimestamp_from_asc() returns
PGTYPES_TS_BAD_TIMESTAMP as errno with 0 as result. Hence, after
applying an offset this prints a timestamp with year 2000. What this
should report in the regression test output is the information about
the error happening, so the output that makes no sense because the
timestamp could not be understood.

+	ts1 = PGTYPEStimestamp_from_asc("AM95000062", NULL);
+	text = PGTYPEStimestamp_to_asc(ts1);
+	printf("timestamp_to_asc4: %s\n", text);
+	PGTYPESchar_free(text);
+
/*	abc-03:10:35-def-02/11/94-gh  */
/*      12345678901234567890123456789 */

It is not entirely the fault of this patch, because the same error is
done a couple of lines above when using an incorrect hour number with
timestamp_to_asc3. The correct thing to do would be in the lines of
what num_test2.pgc does with its check_errno(), I think where we'd
check for the errno returned by PGTYPEStimestamp_from_asc() and
printf() its information in the output of the test. And we should do
that for both the old timestamp_to_asc3 and your new timestamp_to_asc4
if we want to be completely correct.
--
Michael

On Fri, Oct 18, 2024 at 02:51:32PM +0900, Michael Paquier wrote:

It is not entirely the fault of this patch, because the same error is
done a couple of lines above when using an incorrect hour number with
timestamp_to_asc3. The correct thing to do would be in the lines of
what num_test2.pgc does with its check_errno(), I think where we'd
check for the errno returned by PGTYPEStimestamp_from_asc() and
printf() its information in the output of the test. And we should do
that for both the old timestamp_to_asc3 and your new timestamp_to_asc4
if we want to be completely correct.

And here is what I had in mind, checking the error code on top of the
output. The output is really optional, but that maps with 0 with the
offset applied, so I've just kept it.
--
Michael

On Tue, Oct 22, 2024 at 02:53:50PM +0900, Michael Paquier wrote:

And here is what I had in mind, checking the error code on top of the
output. The output is really optional, but that maps with 0 with the
offset applied, so I've just kept it.

I had some time this morning, so I have looked at it again, kept the
patch as-is, and backpatched all the way down (with proper authorship
of course).
--
Michael

Hello,

I would like to clarify two points:

1. Are there any known examples of large open-source projects that actively use the ecpg?

2. Do you think it would be appropriate to assign a CVE for this bug?

Best regards,
Pavel Nekrasov
Fobos-NT

On Thu, Oct 24, 2024 at 12:09:10PM +0300, Павел Некрасов wrote:

I would like to clarify two points:

1. Are there any known examples of large open-source projects that
actively use the ecpg?

Yes, some large organizations use it.

2. Do you think it would be appropriate to assign a CVE for this bug?

Nope, this issue does not qualify as worth a CVE when it comes to
PostgreSQL.
--
Michael

Thank you for the clarification!

Would it be possible to know which specific organizations or projects actively use ecpg?

Best regards,
Pavel Nekrasov
Fobos-NT

El día viernes, octubre 25, 2024 a las 09:23:17 +0300, Павел Некрасов escribió:

Thank you for the clarification!

Would it be possible to know which specific organizations or projects actively use ecpg?

Best regards,
Pavel Nekrasov
Fobos-NT

Our software, a complete Library Management System, uses ECPG from C and
C++ written servers on Linux.

matthias

--
Matthias Apitz, ✉ guru@unixarea.de, http://www.unixarea.de/ +49-176-38902045
Public GnuPG key: http://www.unixarea.de/key.pub

Annalena Baerbock: "We are fighting a war against Russia ..." (25.1.2023)

I, Matthias, I am not at war with Russia.
Я не воюю с Россией.
Ich bin nicht im Krieg mit Russland.

On 2024-Oct-25, Павел Некрасов wrote:

Thank you for the clarification!

Would it be possible to know which specific organizations or projects
actively use ecpg?

It doesn't seem realistic to maintain a complete list, and many
organizations are just not going to tell us or anyone that they're using
it.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"This is a foot just waiting to be shot" (Andrew Dunstan)