Skip to content

Commit

Permalink
BUG: Fix crash when adding %s specifier to color legend label
Browse files Browse the repository at this point in the history
If a string specifier followed by a float specifier were added to the label format string, the SNPRINTF call would crash when interpreting the double as a string pointer.
Fixed by only printing the value to the first specifier, and by validating that the specifier defines a floating point type.

Fix #3802
  • Loading branch information
Sunderlandkyl committed Jan 25, 2023
1 parent 3a8deba commit 91d2923
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 13 deletions.
49 changes: 36 additions & 13 deletions Modules/Loadable/Colors/VTKWidgets/vtkSlicerScalarBarActor.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -184,21 +184,24 @@ void vtkSlicerScalarBarActor::LayoutTicks()
}
else
{
// try to make sure the label format supports a floating point number
// TODO issue 3802: replace with more strict regular expression
vtksys::RegularExpression regExForDouble("%.*[fFgGeE]");
if (regExForDouble.find(this->LabelFormat))
std::string sprintfSpecifier;
std::string prefix;
std::string suffix;
if (vtkSlicerScalarBarActor::ValidateFormatString(
sprintfSpecifier, prefix, suffix, this->LabelFormat, "fFgGeE"))
{
SNPRINTF(labelString, 511, sprintfSpecifier.c_str(), val);
std::string labelStdString = prefix + labelString + suffix;
strcpy(labelString, labelStdString.c_str());
}
else
{
if (!formatWarningPrinted)
{
SNPRINTF(labelString, 511, this->LabelFormat, val);
}
else
{
if (!formatWarningPrinted)
{
vtkWarningMacro("LabelFormat doesn't contain a floating point specifier!" << this->LabelFormat);
formatWarningPrinted = true;
}
vtkWarningMacro("LabelFormat doesn't contain a floating point specifier!" << this->LabelFormat);
formatWarningPrinted = true;
}
}
}
this->P->TextActors[i]->SetInput(labelString);

Expand Down Expand Up @@ -422,3 +425,23 @@ void vtkSlicerScalarBarActor::ConfigureTitle()
? this->P->TitleBox.Posn[1]
: this->P->TitleBox.Posn[1] + this->P->TitleBox.Size[this->P->TL[1]]);
}

//-----------------------------------------------------------------------------
bool vtkSlicerScalarBarActor::ValidateFormatString(std::string& validatedFormat, std::string& prefix, std::string& suffix,
const std::string& requestedFormat, const std::string& typeString)
{
// This regex finds sprintf specifications. Only the first is used to format the value
// Regex from: https://stackoverflow.com/a/8915445
std::string regexString = "%([0-9]\\$)?[+-]?([ 0]|'.{1})?-?[0-9]*(\\.[0-9]+)?[" + typeString + "]";
vtksys::RegularExpression specifierRegex = vtksys::RegularExpression(regexString);
vtksys::RegularExpressionMatch specifierMatch;
if (!specifierRegex.find(requestedFormat.c_str(), specifierMatch))
{
return false;
}

validatedFormat = specifierMatch.match(0);
prefix = requestedFormat.substr(0, specifierMatch.start());
suffix = requestedFormat.substr(specifierMatch.end(), requestedFormat.length() - specifierMatch.end());
return true;
}
6 changes: 6 additions & 0 deletions Modules/Loadable/Colors/VTKWidgets/vtkSlicerScalarBarActor.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ class VTK_SLICER_COLORS_VTKWIDGETS_EXPORT vtkSlicerScalarBarActor
void PrepareTitleText() override;
void ConfigureTitle() override;

/// Parses the requestedFormat string to find a validated format for the types contained in typeString.
/// validatedFormat is set to the first matching sprintf for the input types
/// prefix and suffix are set to the non-matching components of requestedFormat
static bool ValidateFormatString(std::string& validatedFormat, std::string& prefix, std::string& suffix,
const std::string& requestedFormat, const std::string& typeString);

/// flag for setting color name as label
int UseAnnotationAsLabel;

Expand Down

0 comments on commit 91d2923

Please sign in to comment.