Skip to content

Commit 7304c7a

Browse files
committed
avoid integer overflow in getImageData
1 parent f9fcc5f commit 7304c7a

2 files changed

Lines changed: 41 additions & 10 deletions

File tree

src/CanvasRenderingContext2d.cc

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,10 +1021,12 @@ Context2d::GetImageData(const Napi::CallbackInfo& info) {
10211021
Napi::Number zero = Napi::Number::New(env, 0);
10221022
Canvas *canvas = this->canvas();
10231023

1024-
int sx = info[0].ToNumber().UnwrapOr(zero).Int32Value();
1025-
int sy = info[1].ToNumber().UnwrapOr(zero).Int32Value();
1026-
int sw = info[2].ToNumber().UnwrapOr(zero).Int32Value();
1027-
int sh = info[3].ToNumber().UnwrapOr(zero).Int32Value();
1024+
// 64 bit integers for when (1) both sx, sw or sy, sh are negative (2) sx and
1025+
// sy are decreased (3) signs are flipped
1026+
int64_t sx = info[0].ToNumber().UnwrapOr(zero).Int32Value();
1027+
int64_t sy = info[1].ToNumber().UnwrapOr(zero).Int32Value();
1028+
int64_t sw = info[2].ToNumber().UnwrapOr(zero).Int32Value();
1029+
int64_t sh = info[3].ToNumber().UnwrapOr(zero).Int32Value();
10281030

10291031
if (!sw) {
10301032
Napi::Error::New(env, "IndexSizeError: The source width is 0.").ThrowAsJavaScriptException();
@@ -1059,11 +1061,11 @@ Context2d::GetImageData(const Napi::CallbackInfo& info) {
10591061
}
10601062

10611063
// Width and height to actually copy
1062-
int cw = sw;
1063-
int ch = sh;
1064+
int64_t cw = sw;
1065+
int64_t ch = sh;
10641066
// Offsets in the destination image
1065-
int ox = 0;
1066-
int oy = 0;
1067+
int64_t ox = 0;
1068+
int64_t oy = 0;
10671069

10681070
// Clamp the copy width and height if the copy would go outside the image
10691071
if (sx + sw > width) cw = width - sx;
@@ -1083,8 +1085,16 @@ Context2d::GetImageData(const Napi::CallbackInfo& info) {
10831085

10841086
int srcStride = canvas->stride();
10851087
int bpp = srcStride / width;
1086-
int size = sw * sh * bpp;
1087-
int dstStride = sw * bpp;
1088+
// Note: barely fits: INT32_MAX * INT32_MAX * 4. Bpp can't be bigger than 4!
1089+
uint64_t size = (uint64_t)sw * sh * bpp;
1090+
int64_t dstStride = sw * bpp;
1091+
1092+
if (size > INT32_MAX) {
1093+
// INT32_MAX is what Firefox limits the buffer to
1094+
std::string msg = "buffer exceeds " + std::to_string(INT32_MAX) + " bytes";
1095+
Napi::Error::New(env, msg).ThrowAsJavaScriptException();
1096+
return env.Undefined();
1097+
}
10881098

10891099
uint8_t *src = canvas->data();
10901100

test/canvas.test.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,6 +1769,27 @@ describe('Canvas', function () {
17691769
})
17701770
})
17711771

1772+
it('safely rejects integer overflows', function () {
1773+
const canvas = createCanvas(256, 1)
1774+
const ctx = canvas.getContext('2d', {pixelFormat: 'A8'})
1775+
// Integer overflow: 256 * 16777217 * 1(A8) = 0x100000100 > INT32_MAX
1776+
assert.throws(() => ctx.getImageData(0, 0, 256, 16777217))
1777+
// abs(INT32_MIN) * 1 * 1(A8) = 0x80000000 > INT32_MAX
1778+
assert.throws(() => ctx.getImageData(0, 0, -0x80000000, 1))
1779+
// If sx + sw are ints, then when setting the copy width like this:
1780+
//
1781+
// int cw = sw;
1782+
// if (sx + sw > width) cw = width - sx;
1783+
//
1784+
// cw stays as sw, meaning we get 100 bytes of junk/sensitive data
1785+
const dx = ctx.getImageData(0x7fffffff, 0, 100, 1) // hopefully SIGSEGVs
1786+
assert.equal(dx.data.length, 100)
1787+
for (let i = 0; i < dx.data.length; i++) assert.equal(dx.data[i], 0)
1788+
const dy = ctx.getImageData(0, 0x7fffffff, 1, 100) // hopefully SIGSEGVs
1789+
assert.equal(dy.data.length, 100)
1790+
for (let i = 0; i < dy.data.length; i++) assert.equal(dy.data[i], 0)
1791+
});
1792+
17721793
describe('does not throw if rectangle is outside the canvas (#2024)', function () {
17731794
it('on the left', function () {
17741795
const ctx = createTestCanvas(true, { pixelFormat: 'A8' })

0 commit comments

Comments
 (0)