On Wed, Feb 3, 2010 at 2:27 PM, Jeff Garzik <jeff@xxxxxxxxxx> wrote: > On 02/03/2010 08:45 AM, Colin McCabe wrote: >> >> When we create a static buffer for an inode name, and treat it like a >> null-terminated string, it needs to be of length CLD_INODE_NAME_MAX + 1 so >> that it can hold the NULL-terminator. >> >> In cldc_del and cldc_open, we should check that the user-submitted inode >> name >> is less than or equal to CLD_INODE_NAME_MAX. Formerly we were just >> checking >> that it wasn't too big to fit in the packet. >> >> When copying the inode name out of struct cld_dirent_cur, use snprintf >> rather >> than strcpy to ensure that we never overflow the buffer. This isn't >> strictly >> necessary if all other checks are working perfectly, but it seems prudent. >> >> Signed-off-by: Colin McCabe<cmccabe@xxxxxxxxxxxxxx> > > applied, after s/snprintf/strncpy/ > > In general, too, you should never pass a variable string into snprintf, as > that may make a program vulnerable to printf format string attacks (user > supplies "%s" as a username, for example). Oh, how embarassing. I hate it when people do that, now I did it too. I tend to avoid strncpy because of how it always writes n bytes. If only glibc had picked up strlcpy. Colin -- To unsubscribe from this list: send the line "unsubscribe hail-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
