Possible security hole

jw schultz jw at pegasys.ws
Sun Oct 5 12:22:06 EST 2003

On Sat, Oct 04, 2003 at 04:56:16PM -0700, Wayne Davison wrote:
> On Sat, Oct 04, 2003 at 11:38:48PM +0300, Timo Sirainen wrote:
> > 	for (i=0; i < (int) s->count;i++) {
> Yeah, that's pretty bad.  Attached is a patch that should fix this and a
> number of other related problems where the code assumed that size_t
> would fit into an int.  There looks to be a bunch more to do, though.
> For instance, a couple months ago I was starting to look at variable
> sizes after I had noticed that this struct had a bunch of size_t values
> in it that were being stored into int variables for processing:
> struct sum_struct {
>        OFF_T flength;          /**< total file length */
>        size_t count;           /**< how many chunks */
>        size_t remainder;       /**< flength % block_length */
>        size_t blength;         /**< block_length */
>        size_t s2length;        /**< sum2_length */
>        struct sum_buf *sums;   /**< points to info for each chunk */
> };
> The problem variables are remainder, block_length, and s2length.  My
> initial reaction was to resize these three values in the struct to be
> "int"s, but I haven't verified that they're really guaranteed to fit.

They are transmitted as uint32.  In any sane circumstance
they would fit into sint32.  The odd one is s2length which
actually is limited to a range of 2..16 so could have been
stored in an usigned char but padding would have expanded it
in any case.

In truth i'd feel more comfortable with a datatype that was
of a fixed width matching the transmission method.  size_t
ought to be a uint64 on systems supporting 64 bit addresses
but uint32 on 32bit systems.

> Anyway, this patch was prepared pretty quickly, so if anyone notices
> something wrong with it, let me know.

	J.W. Schultz            Pegasystems Technologies
	email address:		jw at pegasys.ws

		Remember Cernan and Schmitt

More information about the rsync mailing list