Re: bug in pgcrypto 0.3

Started by Marko Kreenover 24 years ago5 messages
#1Marko Kreen
marko@l-t.ee

On Sat, May 12, 2001 at 12:47:33AM -0400, Neil Conway wrote:

I've been experimenting with pgcrypto 0.3 (distributed with
Postgres 7.1.0), and I think I've found a bug.

I compiled Pgcrypto with OpenSSL, using gcc 2.95.4 and
OpenSSL 0.9.6a (the latest Debian 'unstable' packages).

web=> select encode(digest('blah', 'sha1'), 'base64');
FATAL 1: pg_encode: overflow, encode estimate too small
pqReadData() -- backend closed the channel unexpectedly.
This probably means the backend terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.

Is this a bug? Can it be fixed?

This is a bug alright. And a silly one :)

Thanks for reporting. For standalone package apply this
patch with -p2.

pgsql-hackers: this should get into REL7_1_STABLE.

--
marko

Index: contrib/pgcrypto/encode.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/contrib/pgcrypto/encode.c,v
retrieving revision 1.4
diff -u -r1.4 encode.c
--- contrib/pgcrypto/encode.c	2001/03/22 03:59:10	1.4
+++ contrib/pgcrypto/encode.c	2001/05/12 08:28:50
@@ -349,7 +349,7 @@
 uint
 b64_enc_len(uint srclen)
 {
-	return srclen + (srclen / 3) + (srclen / (76 / 2));
+	return srclen + (srclen + 2 / 3) + (srclen / (76 / 2)) + 2;
 }

uint

#2Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Marko Kreen (#1)
Re: Re: bug in pgcrypto 0.3

Applied to 7.1.X and 7.2.

On Sat, May 12, 2001 at 12:47:33AM -0400, Neil Conway wrote:

I've been experimenting with pgcrypto 0.3 (distributed with
Postgres 7.1.0), and I think I've found a bug.

I compiled Pgcrypto with OpenSSL, using gcc 2.95.4 and
OpenSSL 0.9.6a (the latest Debian 'unstable' packages).

web=> select encode(digest('blah', 'sha1'), 'base64');
FATAL 1: pg_encode: overflow, encode estimate too small
pqReadData() -- backend closed the channel unexpectedly.
This probably means the backend terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.

Is this a bug? Can it be fixed?

This is a bug alright. And a silly one :)

Thanks for reporting. For standalone package apply this
patch with -p2.

pgsql-hackers: this should get into REL7_1_STABLE.

--
marko

Index: contrib/pgcrypto/encode.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/contrib/pgcrypto/encode.c,v
retrieving revision 1.4
diff -u -r1.4 encode.c
--- contrib/pgcrypto/encode.c	2001/03/22 03:59:10	1.4
+++ contrib/pgcrypto/encode.c	2001/05/12 08:28:50
@@ -349,7 +349,7 @@
uint
b64_enc_len(uint srclen)
{
-	return srclen + (srclen / 3) + (srclen / (76 / 2));
+	return srclen + (srclen + 2 / 3) + (srclen / (76 / 2)) + 2;
}

uint

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#3Ian Lance Taylor
ian@airs.com
In reply to: Bruce Momjian (#2)
Re: Re: bug in pgcrypto 0.3

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Applied to 7.1.X and 7.2.

But, but...

-	return srclen + (srclen / 3) + (srclen / (76 / 2));
+	return srclen + (srclen + 2 / 3) + (srclen / (76 / 2)) + 2;

(srclen + 2 / 3) is always the same as (srclen).

Perhaps this was meant to be ((srclen + 2) / 3)?

The current code is safe, but weird.

Ian

#4Marko Kreen
marko@l-t.ee
In reply to: Ian Lance Taylor (#3)
Re: Re: bug in pgcrypto 0.3

On Mon, May 14, 2001 at 01:15:59PM -0700, Ian Lance Taylor wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Applied to 7.1.X and 7.2.

But, but...

;)

-	return srclen + (srclen / 3) + (srclen / (76 / 2));
+	return srclen + (srclen + 2 / 3) + (srclen / (76 / 2)) + 2;

(srclen + 2 / 3) is always the same as (srclen).

Perhaps this was meant to be ((srclen + 2) / 3)?

I guess too... Its no good to create patches half-asleep...

The current code is safe, but weird.

But I got very good response time :)

Well, the correct code - that corresponds to current
encode - is below. I even got the linefeed stuff wrong.

--
marko

Index: contrib/pgcrypto/encode.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/contrib/pgcrypto/encode.c,v
retrieving revision 1.5
diff -u -r1.5 encode.c
--- contrib/pgcrypto/encode.c	2001/05/13 02:17:09	1.5
+++ contrib/pgcrypto/encode.c	2001/05/14 21:29:43
@@ -349,7 +349,8 @@
 uint
 b64_enc_len(uint srclen)
 {
-	return srclen + (srclen + 2 / 3) + (srclen / (76 / 2)) + 2;
+	/* 3 bytes will be converted to 4, linefeed after 76 chars */
+	return (srclen + 2) * 4 / 3 + srclen / (76 * 3 / 4);
 }

uint

#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Marko Kreen (#4)
Re: Re: bug in pgcrypto 0.3

Applied for 7.1.X and 7.2.

On Mon, May 14, 2001 at 01:15:59PM -0700, Ian Lance Taylor wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Applied to 7.1.X and 7.2.

But, but...

;)

-	return srclen + (srclen / 3) + (srclen / (76 / 2));
+	return srclen + (srclen + 2 / 3) + (srclen / (76 / 2)) + 2;

(srclen + 2 / 3) is always the same as (srclen).

Perhaps this was meant to be ((srclen + 2) / 3)?

I guess too... Its no good to create patches half-asleep...

The current code is safe, but weird.

But I got very good response time :)

Well, the correct code - that corresponds to current
encode - is below. I even got the linefeed stuff wrong.

--
marko

Index: contrib/pgcrypto/encode.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/contrib/pgcrypto/encode.c,v
retrieving revision 1.5
diff -u -r1.5 encode.c
--- contrib/pgcrypto/encode.c	2001/05/13 02:17:09	1.5
+++ contrib/pgcrypto/encode.c	2001/05/14 21:29:43
@@ -349,7 +349,8 @@
uint
b64_enc_len(uint srclen)
{
-	return srclen + (srclen + 2 / 3) + (srclen / (76 / 2)) + 2;
+	/* 3 bytes will be converted to 4, linefeed after 76 chars */
+	return (srclen + 2) * 4 / 3 + srclen / (76 * 3 / 4);
}

uint

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026