Re: [fetchmail-devel] Re: r4000 - trunk

[Matthias A. <ma...@dt...>]

Graham Wilson <gr...@mk...> writes:

>>      if (getuid() == ROOT_UID) {
>>  	lockfile = (char *)xmalloc(
>>  		sizeof(PID_DIR) + sizeof(FETCHMAIL_PIDFILE) + 1);
>> -	sprintf(lockfile, "%s/%s", PID_DIR, FETCHMAIL_PIDFILE);
>> +	strcpy(lockfile, PID_DIR);
>> +	strcat(lockfile, "/");
>> +	strcat(lockfile, FETCHMAIL_PIDFILE);
>>      } else {
>>  	lockfile = (char *)xmalloc(strlen(fmhome)+sizeof(FETCHMAIL_PIDFILE)+2);
>>  	strcpy(lockfile, fmhome);
>
> Why did you switch to strcat (which doesn't check bounds) instead of
> snprintf? I would think we should just change that sprintf call to
> snprintf.

Consistency in these particular if...else branches.

The memory has been allocated anew so we know everything fits.

>> Modified: trunk/unmime.c
>> ===================================================================
>> --- trunk/unmime.c	2004-11-10 20:14:18 UTC (rev 3999)
>> +++ trunk/unmime.c	2004-11-10 20:38:06 UTC (rev 4000)
>> @@ -669,9 +669,9 @@
>>    char fnam[100];
>>  
>>    pid = getpid();
>> -  sprintf(fnam, "/tmp/i_unmime.%x", pid);
>> +  sprintf(fnam, "/tmp/i_unmime.%lx", (long)pid);
>>    fd_orig = fopen(fnam, "w");
>> -  sprintf(fnam, "/tmp/o_unmime.%x", pid);
>> +  sprintf(fnam, "/tmp/o_unmime.%lx", (long)pid);
>>    fd_conv = fopen(fnam, "w");
>>  #endif
>
> These should be changed to snprintf as well I assume?

The buffer holds 100 bytes, we'll stuff at most 31 on 64bit machines or
47 on 128bit machines - and it's only used when the macro STANDALONE is
defined, but not for the module compiled into fetchmail.

We can change the stuff just for completeness though.

-- 
Matthias Andree