On Fri, 2002-11-15 at 19:00, Michael Howard wrote:
> "secure" functions are a complete crok - there is no such thing... There
> are some functions that help you get things right, but they do not lead
> to secure code... Anyone who thinks they can do away with the
> "dangerous" functions and replace them with the "safe" 'n' versions (ie;
> strcpy -> strncpy) is kidding themselves.

I disagree. Using strncpy instead of strcpy goes a long way in
preventing buffer overflows. But you are correct, it isn't a magic
bullet that makes code secure. I wouldn't call strncpy 'crok' though.

> if(m_inst.m_fDesktop) {
> wcsncat( szTmp, L"__DESKTOP", MAXSTRLEN(szTmp) );
> wcsncat( szTmp, szExtSrc , MAXSTRLEN(szTmp) );

Wrong use of wcsncat, you are right. wcsncat and strncat append at a
maximum <count> chars. Above example can lead to an overflow since the
second call doesn't take into account the already used chars in <dst>.
Also, MAXSTRLEN is the size of the buffer. Your use ignores the
terminating \0 that strncat will append)

The correct syntax would be:

> wcsncat( szTmp, L"__DESKTOP", MAXSTRLEN(szTmp)-1 );
> wcsncat( szTmp, szExtSrc , MAXSTRLEN(szTmp) - wcsnlen(szTmp) -1 );

As a side note, proper use of snprintf would be:
ret=snprintf(mystr,sizeof(mystr)-1,"Format: %s",var);

Perhaps we should start development of a standardized 'safe' header file
that can contain macros for snprintf, strncat and the like.

Example:
#define safe_snprinf(dst,len,fmt,var)
snprintf(dst,(len>sizeof(dst)-1)?sizeof(dst)-1:len,fmt,var)

#define safe_strcat(dst,src)
strncat(dst,src,sizeof(dst))

#define safe_strncat(dst,src,len)
strncat(dst,src,(len>sizeof(dst)-1-strlen(dst))?
(sizeof(dst)-1-strlen(dst):len)

Although it may be easier to rewrite the libraries with checks
implemented...
My argument is that we should move security into the libraries and tools
and not rely on the developer to implement checks to avoid existing (but
documented) flaws..

Regards,
Frank

[ reply ]