Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageMaskSpatialObject tries to access pixels outside the image buffer (segfault) #1950

Closed
N-Dekker opened this issue Aug 6, 2020 · 0 comments
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Milestone

Comments

@N-Dekker
Copy link
Contributor

N-Dekker commented Aug 6, 2020

Description

ImageMaskSpatialObject::IsInsideInObjectSpace does in some border cases try to access pixels outside the image buffer, which sometimes triggers a segmentation fault, or an incorrect result. This issue appears related to some specific elastix 5.0.0 segfaults reported by Theo van Walsum (@tvanwalsum, Erasmus MC, Rotterdam) at Google Groups - elastix-imageregistration - Issues with mask in el 5.0.0 when mask extent is smaller then image extent.

Steps to Reproduce Assert Failure

#include <itkImage.h>
#include <itkSize.h>
#include <itkImageMaskSpatialObject.h>
#include <cassert>
#include <cstdlib>

int main()
{
	const auto image = itk::Image<unsigned char>::New();
	image->SetRegions(itk::Size<>{ {2, 2} });
	image->Allocate(true);
	const auto imageMaskSpatialObject = itk::ImageMaskSpatialObject<2>::New();
	imageMaskSpatialObject->SetImage(image);
	const double point[] = { 1.5, 1.5 };
	if (imageMaskSpatialObject->IsInsideInObjectSpace(point))
	{
		assert(!"Incorrectly estimated that the specified point is inside the mask!");
		return EXIT_FAILURE;
	}
	return EXIT_SUCCESS;
}

Expected behavior

return EXIT_SUCCESS.

Actual behavior

assert failure, return EXIT_FAILURE.

Steps to Reproduce Segfault

#include <itkImage.h>
#include <itkSize.h>
#include <itkImageMaskSpatialObject.h>

int main()
{
	const auto image = itk::Image<unsigned char, 3>::New();
	const itk::Size<3> imageSize{ {256, 128, 32} };
	image->SetRegions(imageSize);
	const double origin[] = { 0.0, -16.0, 0.0 };
	image->SetOrigin(origin);
	image->Allocate(true);

	const auto imageMaskSpatialObject = itk::ImageMaskSpatialObject<>::New();
	imageMaskSpatialObject->SetImage(image);
	const double point[] = { 0.0, 0.0, 31.5 };
	imageMaskSpatialObject->IsInsideInObjectSpace(point); // read access violation!
}

Reproducibility

Segmentation faults only seem to happen for specific combinations of image size, origin, and point coordinates.

Versions

Reproduced with ITK v5.0.1, as well as the current version (August 6, 2020) at the master: a1a7e0b

Environment

Visual Studio 2019 (Debug). Note that the issue does not seem to be specific to Visual Studio.

Additional Information

There is a possible fix at pull request #1951

@N-Dekker N-Dekker added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Aug 6, 2020
N-Dekker added a commit to N-Dekker/ITK that referenced this issue Aug 6, 2020
Based on example code from issue InsightSoftwareConsortium#1950, "ImageMaskSpatialObject tries
to access pixels outside the image buffer (segfault)"
@thewtex thewtex added this to the ITK 5.1.1 milestone Aug 6, 2020
N-Dekker added a commit to N-Dekker/ITK that referenced this issue Aug 6, 2020
… access outside image buffer

Fixed issue InsightSoftwareConsortium#1950, "ImageMaskSpatialObject tries to access pixels
outside the image buffer (segfault)".

In the original code, `TransformPhysicalPointToContinuousIndex` did
estimate whether the specified point was inside the image buffer, but
then `GetInterpolator()->EvaluateAtContinuousIndex(index)` did in some
cases still try to access a pixel outside the image buffer.

This fix avoids using an interpolator. It only accesses a pixel when its
index is inside the buffered region.

The use of an interpolator appears less relevant for an image mask than
for other spatial objects, as for each mask pixel value, it is usually
only interesting to know whether it is zero or non-zero.

Added ImageMaskSpatialObject.CornerPointIsNotInsideMaskOfZeroValues unit
test.
thewtex pushed a commit that referenced this issue Aug 10, 2020
Fixed issue #1950, "ImageMaskSpatialObject tries to access pixels
outside the image buffer (segfault)".

In the original code, `TransformPhysicalPointToContinuousIndex` did
estimate whether the specified point was inside the image buffer, but
then `GetInterpolator()->EvaluateAtContinuousIndex(index)` did in some
cases still try to access a pixel outside the image buffer.

This fix avoids using an interpolator. It only accesses a pixel when its
index is inside the buffered region.

The use of an interpolator appears less relevant for an image mask than
for other spatial objects, as for each mask pixel value, it is usually
only interesting to know whether it is zero or non-zero.

Added ImageMaskSpatialObject.CornerPointIsNotInsideMaskOfZeroValues unit
test.
@thewtex thewtex closed this as completed Aug 10, 2020
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

No branches or pull requests

2 participants