Thread: pgcrypto bug

When given oversized key, encrypt/decrypt corrupted
memory.  This fixes it.  Also a free() was missing.

--
marko

Index: contrib/pgcrypto/px.c
===================================================================
RCS file: /opt/cvs/pgsql/pgsql/contrib/pgcrypto/px.c,v
retrieving revision 1.3
diff -u -r1.3 px.c
--- contrib/pgcrypto/px.c    25 Oct 2001 05:49:20 -0000    1.3
+++ contrib/pgcrypto/px.c    7 Nov 2001 22:33:44 -0000
@@ -88,6 +88,8 @@
             memcpy(ivbuf, iv, ivlen);
     }

+    if (klen > ks)
+        klen = ks;
     keybuf = px_alloc(ks);
     memset(keybuf, 0, ks);
     memcpy(keybuf, key, klen);
@@ -96,6 +98,7 @@

     if (ivbuf)
         px_free(ivbuf);
+    px_free(keybuf);

     return err;
 }

Bug fix.  Patch applied.  Thanks.

---------------------------------------------------------------------------


> When given oversized key, encrypt/decrypt corrupted
> memory.  This fixes it.  Also a free() was missing.
>
> --
> marko
>
> Index: contrib/pgcrypto/px.c
> ===================================================================
> RCS file: /opt/cvs/pgsql/pgsql/contrib/pgcrypto/px.c,v
> retrieving revision 1.3
> diff -u -r1.3 px.c
> --- contrib/pgcrypto/px.c    25 Oct 2001 05:49:20 -0000    1.3
> +++ contrib/pgcrypto/px.c    7 Nov 2001 22:33:44 -0000
> @@ -88,6 +88,8 @@
>              memcpy(ivbuf, iv, ivlen);
>      }
>
> +    if (klen > ks)
> +        klen = ks;
>      keybuf = px_alloc(ks);
>      memset(keybuf, 0, ks);
>      memcpy(keybuf, key, klen);
> @@ -96,6 +98,7 @@
>
>      if (ivbuf)
>          px_free(ivbuf);
> +    px_free(keybuf);
>
>      return err;
>  }
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

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

Hi all,

attached is a patch which kills a race condition in the current Makefile
system. The Problem occurs if you compile with make -j3 (as many ppl
with Dual CPU Systems will do).
Removing the y.tab.X file is not a good idea as this will trigger
a second compile in the make install stage ...

    Bye, Klaus

--
Klaus Naumann
Systems Administration
GMX  Aktiengesellschaft
Riesstrasse 17, 80992 München
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Klaus Naumann <knaumann@gmx-ag.de> writes:
> attached is a patch which kills a race condition in the current Makefile
> system. The Problem occurs if you compile with make -j3 (as many ppl
> with Dual CPU Systems will do).
> Removing the y.tab.X file is not a good idea as this will trigger
> a second compile in the make install stage ...

Why?  y.tab.X are not make targets, only intermediate files, so I don't
see why removing them will create any problem.  And I don't see why
turning them into make targets as your patch does is a good idea.

            regards, tom lane

On Thu, 8 Nov 2001, Tom Lane wrote:

> Klaus Naumann <knaumann@gmx-ag.de> writes:
> > attached is a patch which kills a race condition in the current Makefile
> > system. The Problem occurs if you compile with make -j3 (as many ppl
> > with Dual CPU Systems will do).
> > Removing the y.tab.X file is not a good idea as this will trigger
> > a second compile in the make install stage ...
>
> Why?  y.tab.X are not make targets, only intermediate files, so I don't
> see why removing them will create any problem.  And I don't see why
> turning them into make targets as your patch does is a good idea.

It is ... the problem is that with the current Makefile there
are always two files which need the .y files
(i.e. preproc.c and preproc.h). If you now do a make -j3,
make will start things in parallel. This means that bison is called twice
(it can even be called the same time which corrupts the output
y.tab.* files) and if you mv the files away the second call
failes as the second mv can't find the file anymore.
Even a test -f y.tab.c && mv doesn't help as I have
seen cases when the race was that bad that test found the
file but mv didn't anymore ...

So the solution is: Turn the bison and mv part into a single
target which get's called once.
This involves making y.tab.c a target. And to avoid a second
compilation on make install it's necessary to not remove
those files (turn mv into cp).

Also I don't see why keeping the y.tab.* files is a problem.
Neither are they big, nor are they installed or anything.
They are like a .o file ...

    Later, Klaus

--
Klaus Naumann
Systems Administration
GMX  Aktiengesellschaft
Riesstrasse 17, 80992 München
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Klaus Naumann <knaumann@gmx-ag.de> writes:
> It is ... the problem is that with the current Makefile there
> are always two files which need the .y files
> (i.e. preproc.c and preproc.h). If you now do a make -j3,
> make will start things in parallel.

But they're generated by the *same action*.  If make runs the same
action twice in parallel, it's make that is broken.  I think your
gripe is directed to the wrong place.

            regards, tom lane

Klaus Naumann writes:

> attached is a patch which kills a race condition in the current Makefile
> system. The Problem occurs if you compile with make -j3 (as many ppl
> with Dual CPU Systems will do).
> Removing the y.tab.X file is not a good idea as this will trigger
> a second compile in the make install stage ...

Well, -j does work if you're using a tarball, but not in a bare CVS tree.

There are a couple of details that I don't like about your patch, but I
can see that the issue gets fixed.

--
Peter Eisentraut   peter_e@gmx.net

On Fri, 9 Nov 2001, Peter Eisentraut wrote:

> Klaus Naumann writes:
>
> > attached is a patch which kills a race condition in the current Makefile
> > system. The Problem occurs if you compile with make -j3 (as many ppl
> > with Dual CPU Systems will do).
> > Removing the y.tab.X file is not a good idea as this will trigger
> > a second compile in the make install stage ...
>
> Well, -j does work if you're using a tarball, but not in a bare CVS tree.

Why is that - or better, what's changed in the tar ball which makes it
work ?

> There are a couple of details that I don't like about your patch, but I
> can see that the issue gets fixed.

What are the details ?

    Thanks, Klaus

--
Klaus Naumann
Systems Administration
GMX  Aktiengesellschaft
Riesstrasse 17, 80992 München
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/