[Insight-developers] formatstring vulnerability in NumericSeriesFileNames::GetFileNames

[Kent Williams]

On platforms that support it. substituting snprintf will remove the 
buffer overflow problem.
I checked, and Linux,MacOSX and Irix support snprintf. Not all Windows 
compilers support snprintf, though Visual C++ and Borland C++ do have 
_snprintf going back several versions, which is the same thing.

It would be a worthy goal to remove printf family functions from 
Insight, both because of the potential for buffer overruns, and because 
printf isn't part of "The C++ Way."  Unfortunately printf-family 
functions seem to provide finer-grained control over output formats than 
their C++ replacements, or at least I've never seen a concise 
description of how to replace printf with functionally equivalent stream 
operations.

It probably wouldn't hurt to add a snprintf wrapper to the KWSys 
library, to hide compiler dependencies. In looking through the KWSys 
header, I noticed there is a function EstimateFormatLength, which will 
return the upper bound on the buffer size needed for a particular format 
and argument list. Unfortunately it uses va_list argument lists, which 
are rather clumsy to use.

Boost has a replacement, but due to the Boost build system, if you want 
to use Boost, you have to live in Boost's world.

Henning Meyer wrote:

> Hello,
>
> I just had a look a this function and it looks like there is a format
> string vulnerability:
>  char temp[4096];
>  for (unsigned long i = m_StartIndex; i <= m_EndIndex; i+= 
> m_IncrementIndex)
>    {
>    sprintf (temp, m_SeriesFormat.c_str(), i);
>    std::string fileName(temp);
>    m_FileNames.push_back(fileName);
>    }
> As far as I have seen the length of m_SeriesFormat is not checked. So
> one might use it for format string attacks.