I attach patch which adds boundaries check and memory overwriting protection when compressed data are corrupted.


Current behavior let code overwrite a memory and after that check if unpacked size is same as expected value. In this case elog execution fails (at least on Solaris - malloc has corrupted structures) and no message appears in a log file.

I did not add any extra information into the message. Reasonable solution seems to be use errcontext how was recommended by Alvaro. But I 'm not sure if printtup is good place for it, because pg_detoast is called from many places. However, is can be solved in separate patch.

I'm also think that this modification should be backported to other version too.

                Thanks Zdenek
Index: src/backend/utils/adt/pg_lzcompress.c
===================================================================
RCS file: /zfs_data/cvs_pgsql/cvsroot/pgsql/src/backend/utils/adt/pg_lzcompress.c,v
retrieving revision 1.29
diff -c -r1.29 pg_lzcompress.c
*** src/backend/utils/adt/pg_lzcompress.c	1 Jan 2008 19:45:52 -0000	1.29
--- src/backend/utils/adt/pg_lzcompress.c	29 Feb 2008 19:07:50 -0000
***************
*** 633,638 ****
--- 633,639 ----
  {
  	const unsigned char *dp;
  	const unsigned char *dend;
+ 	const unsigned char *destend;
  	unsigned char *bp;
  	unsigned char ctrl;
  	int32		ctrlc;
***************
*** 643,656 ****
  	dp = ((const unsigned char *) source) + sizeof(PGLZ_Header);
  	dend = ((const unsigned char *) source) + VARSIZE(source);
  	bp = (unsigned char *) dest;
  
! 	while (dp < dend)
  	{
  		/*
  		 * Read one control byte and process the next 8 items.
  		 */
  		ctrl = *dp++;
! 		for (ctrlc = 0; ctrlc < 8 && dp < dend; ctrlc++)
  		{
  			if (ctrl & 1)
  			{
--- 644,658 ----
  	dp = ((const unsigned char *) source) + sizeof(PGLZ_Header);
  	dend = ((const unsigned char *) source) + VARSIZE(source);
  	bp = (unsigned char *) dest;
+ 	destend = ((const unsigned char *)dest) + source->rawsize;
  
! 	while (dp < dend && bp < destend)
  	{
  		/*
  		 * Read one control byte and process the next 8 items.
  		 */
  		ctrl = *dp++;
! 		for (ctrlc = 0; ctrlc < 8 && dp < dend && bp < destend; ctrlc++)
  		{
  			if (ctrl & 1)
  			{
***************
*** 673,678 ****
--- 675,682 ----
  				 * memcpy() here, because the copied areas could overlap
  				 * extremely!
  				 */
+ 				if( bp+len > destend) /* data are corrupted, do not continue */
+ 					break;
  				while (len--)
  				{
  					*bp = bp[-off];
***************
*** 696,708 ****
  	}
  
  	/*
! 	 * Check we decompressed the right amount, else die.  This is a FATAL
! 	 * condition if we tromped on more memory than expected (we assume we have
! 	 * not tromped on shared memory, though, so need not PANIC).
  	 */
! 	destsize = (char *) bp - dest;
! 	if (destsize != source->rawsize)
! 		elog(destsize > source->rawsize ? FATAL : ERROR,
  			 "compressed data is corrupt");
  
  	/*
--- 700,709 ----
  	}
  
  	/*
! 	 * Check we decompressed the right amount.
  	 */
! 	if (bp != destend || dp != dend)
! 		elog(ERROR,
  			 "compressed data is corrupt");
  
  	/*
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Reply via email to