Re: [Hamlib-developer] sprintf's in code/Overflow risk

[Stelios B. <sb...@en...>]

Hi Charles,

>>>>> On Fri, 13 Nov 2009 17:33:36 -0500, Charles Suprin <ham...@gm...> said:

[snip]

> On Thu, Nov 12, 2009 at 10:53 PM, Stelios Bounanos <sb...@en...>wrote:

>> > +#define SAFER_SPRINTF(x,y,z) {if
>> (snprintf((x),sizeof(x),(y),(z))==sizeof(x)) {return -RIG_EINTERNAL;}}
>> 
>> #define SAFER_SPRINTF(x, ...) {if
>> (snprintf((x),sizeof(x),__VA_ARGS__)>=sizeof(x)) {return -RIG_EINTERNAL;}}
>> 
>> so that it will work with more than two arguments (note the >=
>> too).  Also it only works for arrays.
>> 
> There are several points here, so I will try to address them one at a time.

> Yes this only works for arrays.  This is all that is required here.  However
> it would be nice to have something that worked for a more general solution.
> It also would be nice if someone tried to use a pointer the compiler would
> throw a processor exception.  However I am not a master of the preprocessor
> judo. If this were C++ it would not be an issue, but here we are and I do
> not have a nicer solution.

> The use of ellipsis here (...) was something I could not decide whether it
> was the right answer or not. It was not clear whether for portability and
> security it was worthwhile. None of the applications of this macro required
> it and I have worked with some embedded compiles that do not support it.
> (...) came about in C99.  Security is a little bit of a weaker position, but
> if the compiler throws an error if someone uses it incorrectly it seems a
> better idea.  Again, the right answer is not obvious to me.

The (...) will make the macro behave more like sprintf.  You will still get
whatever type checks the compiler can do.  For example, the glibc prototypes use
__attribute__ ((format(TYPE, INDEX, FIRST))) and gcc can check the argument
types.

Also, hamlib already requires C99 because it uses designated initializers in
struct definitions.

>> +#define SAFER_STRCPY(x,y) {if (strlen(y)>sizeof(x)) {return
>> -RIG_EINTERNAL;} else {strncpy((x),(y),sizeof(x));}}
>> 
>> Again you'd have to be careful to never use this when x is a char* and
>> not a char[].  If it must work with pointers you have to check strlen(y)
>> and strlen(x), and then you might as well use memcpy for the copying.
>> 
>> [snip]
>> 
> Again, same discussion with char* versus char[] as above.

> Now on the use of strlen I am confused.  strlen only tells you how many
> characters are in the order before it reaches the string terminatino
> character.  This may be before the end of the buffer or after the end of the
> buffer.  If in c++ we could query the length of the string buffer but this
> being c it needs to be passed.

My argument was that if it must work with pointers, and if you want to check
before you start copying, you must call strlen twice. strncpy works when its
arguments are pointers by assuming that the offset of the first \0 is also the
length of the string.


-- 

73,
Stelios, M0GLD.