Skip to content

Commit f49a011

Browse files
Hakerh400zbjornson
authored andcommitted
Prevent segfaults caused by creating a too large canvas (#1151)
* Fix warning * Detect bad allocations * Fix zero-width canvas edge case * Add test * Update changelog * Fix compiler error * Improve error handling * Update tests * Fix tests for node 4 * Add new line
1 parent 7695fb2 commit f49a011

File tree

7 files changed

+112
-42
lines changed

7 files changed

+112
-42
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ project adheres to [Semantic Versioning](http://semver.org/).
2929
* Prevent JPEG errors from crashing process (#1124)
3030
* Improve handling of invalid arguments (#1129)
3131
* Fix repeating patterns when drawing a canvas to itself (#1136)
32+
* Prevent segfaults caused by creating a too large canvas
3233

3334
### Added
3435
* Prebuilds (#992)

src/Canvas.cc

+5
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ NAN_METHOD(Canvas::New) {
116116
backend = new ImageBackend(0, 0);
117117
}
118118

119+
if (!backend->isSurfaceValid()) {
120+
delete backend;
121+
return Nan::ThrowError(backend->getError());
122+
}
123+
119124
Canvas* canvas = new Canvas(backend);
120125
canvas->Wrap(info.This());
121126

src/CanvasRenderingContext2d.cc

+13-6
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,7 @@ NAN_METHOD(Context2d::New) {
675675
}
676676

677677
Context2d *context = new Context2d(canvas);
678+
678679
context->Wrap(info.This());
679680
info.GetReturnValue().Set(info.This());
680681
}
@@ -825,7 +826,7 @@ NAN_METHOD(Context2d::PutImageData) {
825826
dst += dstStride;
826827
src += srcStride;
827828
}
828-
break;
829+
break;
829830
}
830831
case CAIRO_FORMAT_RGB24: {
831832
src += sy * srcStride + sx * 4;
@@ -923,6 +924,14 @@ NAN_METHOD(Context2d::GetImageData) {
923924
if (!sh)
924925
return Nan::ThrowError("IndexSizeError: The source height is 0.");
925926

927+
int width = canvas->getWidth();
928+
int height = canvas->getHeight();
929+
930+
if (!width)
931+
return Nan::ThrowTypeError("Canvas width is 0");
932+
if (!height)
933+
return Nan::ThrowTypeError("Canvas height is 0");
934+
926935
// WebKit and Firefox have this behavior:
927936
// Flip the coordinates so the origin is top/left-most:
928937
if (sw < 0) {
@@ -934,8 +943,6 @@ NAN_METHOD(Context2d::GetImageData) {
934943
sh = -sh;
935944
}
936945

937-
int width = canvas->getWidth();
938-
int height = canvas->getHeight();
939946
if (sx + sw > width) sw = width - sx;
940947
if (sy + sh > height) sh = height - sy;
941948

@@ -955,7 +962,7 @@ NAN_METHOD(Context2d::GetImageData) {
955962
}
956963

957964
int srcStride = canvas->stride();
958-
int bpp = srcStride / canvas->getWidth();
965+
int bpp = srcStride / width;
959966
int size = sw * sh * bpp;
960967
int dstStride = sw * bpp;
961968

@@ -1211,8 +1218,8 @@ NAN_METHOD(Context2d::DrawImage) {
12111218
}
12121219

12131220
bool sameCanvas = surface == context->canvas()->surface();
1214-
cairo_surface_t *surfTemp;
1215-
cairo_t *ctxTemp;
1221+
cairo_surface_t *surfTemp = NULL;
1222+
cairo_t *ctxTemp = NULL;
12161223

12171224
if (sameCanvas) {
12181225
int width = context->canvas()->getWidth();

src/backend/Backend.cc

+48-31
Original file line numberDiff line numberDiff line change
@@ -2,88 +2,105 @@
22

33

44
Backend::Backend(string name, int width, int height)
5-
: name(name)
6-
, width(width)
7-
, height(height)
8-
, surface(NULL)
9-
, canvas(NULL)
10-
, _closure(NULL)
5+
: name(name)
6+
, width(width)
7+
, height(height)
8+
, surface(NULL)
9+
, canvas(NULL)
10+
, _closure(NULL)
1111
{}
1212

1313
Backend::~Backend()
1414
{
15-
this->destroySurface();
15+
this->destroySurface();
1616
}
1717

1818

1919
void Backend::setCanvas(Canvas* _canvas)
2020
{
21-
this->canvas = _canvas;
21+
this->canvas = _canvas;
2222
}
2323

2424

2525
cairo_surface_t* Backend::recreateSurface()
2626
{
27-
this->destroySurface();
27+
this->destroySurface();
2828

29-
return this->createSurface();
29+
return this->createSurface();
3030
}
3131

3232
cairo_surface_t* Backend::getSurface() {
33-
if (!surface) createSurface();
34-
return surface;
33+
if (!surface) createSurface();
34+
return surface;
3535
}
3636

3737
void Backend::destroySurface()
3838
{
39-
if(this->surface)
40-
{
41-
cairo_surface_destroy(this->surface);
42-
this->surface = NULL;
43-
}
39+
if(this->surface)
40+
{
41+
cairo_surface_destroy(this->surface);
42+
this->surface = NULL;
43+
}
4444
}
4545

4646

4747
string Backend::getName()
4848
{
49-
return name;
49+
return name;
5050
}
5151

5252
int Backend::getWidth()
5353
{
54-
return this->width;
54+
return this->width;
5555
}
5656
void Backend::setWidth(int width_)
5757
{
58-
this->width = width_;
59-
this->recreateSurface();
58+
this->width = width_;
59+
this->recreateSurface();
6060
}
6161

6262
int Backend::getHeight()
6363
{
64-
return this->height;
64+
return this->height;
6565
}
6666
void Backend::setHeight(int height_)
6767
{
68-
this->height = height_;
69-
this->recreateSurface();
68+
this->height = height_;
69+
this->recreateSurface();
70+
}
71+
72+
bool Backend::isSurfaceValid(){
73+
bool hadSurface = surface != NULL;
74+
bool isValid = true;
75+
76+
cairo_status_t status = cairo_surface_status(getSurface());
77+
78+
if (status != CAIRO_STATUS_SUCCESS) {
79+
error = cairo_status_to_string(status);
80+
isValid = false;
81+
}
82+
83+
if (!hadSurface)
84+
destroySurface();
85+
86+
return isValid;
7087
}
7188

7289

7390
BackendOperationNotAvailable::BackendOperationNotAvailable(Backend* backend,
74-
string operation_name)
75-
: backend(backend)
76-
, operation_name(operation_name)
91+
string operation_name)
92+
: backend(backend)
93+
, operation_name(operation_name)
7794
{};
7895

7996
BackendOperationNotAvailable::~BackendOperationNotAvailable() throw() {};
8097

8198
const char* BackendOperationNotAvailable::what() const throw()
8299
{
83-
std::ostringstream o;
100+
std::ostringstream o;
84101

85-
o << "operation " << this->operation_name;
86-
o << " not supported by backend " + backend->getName();
102+
o << "operation " << this->operation_name;
103+
o << " not supported by backend " + backend->getName();
87104

88-
return o.str().c_str();
105+
return o.str().c_str();
89106
};

src/backend/Backend.h

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class Backend : public Nan::ObjectWrap
1717
{
1818
private:
1919
const string name;
20+
const char* error = NULL;
2021

2122
protected:
2223
int width;
@@ -58,6 +59,9 @@ class Backend : public Nan::ObjectWrap
5859
return CAIRO_FORMAT_INVALID;
5960
#endif
6061
}
62+
63+
bool isSurfaceValid();
64+
inline const char* getError(){ return error; }
6165
};
6266

6367

src/backend/ImageBackend.cc

+5-5
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ int32_t ImageBackend::approxBytesPerPixel() {
3737

3838
cairo_surface_t* ImageBackend::createSurface()
3939
{
40-
assert(!this->surface);
41-
this->surface = cairo_image_surface_create(this->format, width, height);
42-
assert(this->surface);
43-
Nan::AdjustExternalMemory(approxBytesPerPixel() * width * height);
40+
assert(!this->surface);
41+
this->surface = cairo_image_surface_create(this->format, width, height);
42+
assert(this->surface);
43+
Nan::AdjustExternalMemory(approxBytesPerPixel() * width * height);
4444

45-
return this->surface;
45+
return this->surface;
4646
}
4747

4848
cairo_surface_t* ImageBackend::recreateSurface()

test/canvas.test.js

+36
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,42 @@ describe('Canvas', function () {
257257
assert.ok('object' == typeof ctx);
258258
assert.equal(canvas, ctx.canvas, 'context.canvas is not canvas');
259259
assert.equal(ctx, canvas.context, 'canvas.context is not context');
260+
261+
const MAX_IMAGE_SIZE = 32767;
262+
263+
[
264+
[0, 0, 1],
265+
[1, 0, 1],
266+
[MAX_IMAGE_SIZE, 0, 1],
267+
[MAX_IMAGE_SIZE + 1, 0, 3],
268+
[MAX_IMAGE_SIZE, MAX_IMAGE_SIZE, null],
269+
[MAX_IMAGE_SIZE + 1, MAX_IMAGE_SIZE, 3],
270+
[MAX_IMAGE_SIZE + 1, MAX_IMAGE_SIZE + 1, 3],
271+
[Math.pow(2, 30), 0, 3],
272+
[Math.pow(2, 30), 1, 3],
273+
[Math.pow(2, 32), 0, 1],
274+
[Math.pow(2, 32), 1, 1],
275+
].forEach(params => {
276+
var width = params[0];
277+
var height = params[1];
278+
var errorLevel = params[2];
279+
280+
var level = 3;
281+
282+
try {
283+
var canvas = createCanvas(width, height);
284+
level--;
285+
286+
var ctx = canvas.getContext('2d');
287+
level--;
288+
289+
ctx.getImageData(0, 0, 1, 1);
290+
level--;
291+
} catch (err) {}
292+
293+
if (errorLevel !== null)
294+
assert.strictEqual(level, errorLevel);
295+
});
260296
});
261297

262298
it('Canvas#getContext("2d", {pixelFormat: string})', function () {

0 commit comments

Comments
 (0)