Dear All,

I think I found an array overrun bug in _Slony_I_logTrigger() function
(file: src/backend/slony1_funcs.c). The problematic array is "attkind"
trigger parameter, which is a zero-terminated string containing
letters 'k' and 'v'. These letters determine if the given table column
is a key or an ordinary value. In Slony-I 1.x this string had the
length equal to the number of the columns of the table. However,
starting from Slony-I 2.0.0, this string is trimmed and the trailing
'v' letters are removed. Here is the code from
determineAttkindUnique(text, name) function (file:
src/backend/slony1_funcs.sql, line 4807):

	-- Strip off trailing v characters as they are not needed by the logtrigger
	v_attkind :=3D pg_catalog.rtrim(v_attkind, 'v');

As a result, the "attkind" parameter passed to the
_Slony_I_logTrigger() trigger function is usually shorter than the
number of colunms of the table, but this fact is not taken into
account in the function code. Here is an example (slony1_funcs.c, line
688, but the same is in the loop beginning in line 758):

	for (i =3D 0, attkind_idx =3D -1; i < tg->tg_relation->rd_att->natts; i++)
	{
		if (tupdesc->attrs[i]->attisdropped)
			continue;
		attkind_idx++;
		if (attkind[attkind_idx] !=3D 'k')
			continue;
		/* The rest of the loop... */
	}

These loops iterate over all non-dropped columns of the table and
increment attkind_idx. Since the attkind can be shorter than the
number of columns, the attkind[attkind_idx] expression can read bytes
behind the end of the array. These bytes could have any random value,
including 0x6B (the ASCII code of 'k'), which could lead to random
treatment of an ordinary column as a key column (in fact, we observed
that in our test environment and that's why I investigated the issue).

I attached a small patch against slony1_funcs.c which checks for the 0
byte at the end of the attkind string and stops the iteration. This
patch also adds some error checking in the loop beginning in line 648.
Of course the problem can be removed by not trimming the string in the
determineAttkindUnique(text, name) function, but I think that the end
of a zero-terminated string should be checked in any case.

Regards,

-- =

Adam Buraczewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: slony1_funcs.c.diff.gz
Type: application/x-gzip
Size: 459 bytes
Desc: not available
Url : http://lists.slony.info/pipermail/slony1-bugs/attachments/20081212/59=
e81ffd/slony1_funcs.c.diff.bin