Horton

Thread

Commits

  1. ecpg: Fix out-of-bound read in DecodeDateTime()

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

    The Post Office <noreply@postgresql.org> — 2024-09-12T08:54:42Z

    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"

Patch:

--- a/src/interfaces/ecpg/pgtypeslib/dt_common.c
+++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c
@@ -2327,10 +2327,9 @@ DecodeDateTime(char **field, int *ftype, int nf,
 			return ((fmask & DTK_TIME_M) == DTK_TIME_M) ? 1 : -1;
 
 		/*
-		 * check for valid day of month, now that we know for sure the month
-		 * and year...
+		 * check for valid day of month and month, now that we know for sure the
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;
 
 		/*

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

    Bruce Momjian <bruce@momjian.us> — 2024-10-14T21:52:59Z

    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?"

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

    Michael Paquier <michael@paquier.xyz> — 2024-10-16T00:18:10Z

    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

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

    Bruce Momjian <bruce@momjian.us> — 2024-10-17T17:29:03Z

    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?"

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

    Michael Paquier <michael@paquier.xyz> — 2024-10-18T05:51:32Z

    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

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

    Michael Paquier <michael@paquier.xyz> — 2024-10-22T05:53:50Z

    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

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

    Michael Paquier <michael@paquier.xyz> — 2024-10-22T23:37:35Z

    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

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

    Павел Некрасов <p.nekrasov@fobos-nt.ru> — 2024-10-24T09:09:10Z

    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

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

    Michael Paquier <michael@paquier.xyz> — 2024-10-24T22:25:26Z

    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

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

    Павел Некрасов <p.nekrasov@fobos-nt.ru> — 2024-10-25T06:23:17Z

    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

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

    Matthias Apitz <guru@unixarea.de> — 2024-10-25T09:39:32Z

    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.

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2024-10-26T14:14:53Z

    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)