Automattic / Automattic/node-canvas
putImageData segfaults if the given array is too small
- Dominant language
- JavaScript
- Stars
- 10.7k
- Forks
- 1.2k
- Avg merge
- 4d 8h
- Merged PRs (30d)
- 1
Description
## Issue or Feature
Hi all,
A story of user stupidity: I'm tinkering with raw buffers representing RGB pixels piped from `ffpmeg` for a small hobby CCTV project. My assumption for a canvas context with `{ alpha: false, pixelFormat: 'RGB24' }` was for the `createImageData`/`putImageData` functions to act in a pure RGB mode, but peeking at the C++ source it seems it needs RGBA, but then goes on and ignores the A for the underlying implementation.
That's all fine now I know, but the actual behaviour for a buffer that's 3/4 of the expected size resulted in the entire Node process terminating via a segfault. I think the expected behaviour should be some sort of bounds checking and an exception? I'm not sure whether `createImageData` or `putImageData` should be the one checking. Unfortunately my knowledge of C++ is embarrassingly low, otherwise I would offer to help!
This also doesn't show until the difference is large enough I believe. While creating the reproduction code a square of 40 pixels _didn't_ crash, but shifting it to my original 1280x720 seems to reproduce it reliably.
## Steps to Reproduce
```js
const { createCanvas, createImageData } = require('canvas')
const width = 1280
const height = 720
// Generate some RGB pixels, all of the same colour.
let rawPixels = new Uint8ClampedArray(width * height * 3).fill(200)
console.log(`Passing in ${rawPixels.length} subpixels.`)
const canvas = createCanvas(width, height)
const ctx = canvas.getContext('2d', { alpha: false, pixelFormat: 'RGB24' })
const canvasBufferSize = ctx.getImageData(0, 0, width, height).data.length
console.log(`Filling this canvas requires ${canvasBufferSize} subpixels.`)
const image = createImageData(rawPixels, width, height)
ctx.putImageData(image, 0, 0)
canvas.toBuffer()
// Segfault.
// Doing a `getImageData` shows the buffer length
// of the canvas is actually 1280 * 720 * 4.
// But I was silly and fed it 1280 * 720 * 3...
```
## Your Environment
* Version of node-canvas (output of `npm list canvas` or `yarn list canvas`): 2.6.1
* Environment (e.g. node 4.2.0 on Mac OS X 10.8): Node v10.19.0 on Ubuntu 20.04 amd64. (Also occurred on Node v14 on Windows)
(P.S. I've now implemented a loop in JS to create a new buffer with a zeroed A channel but this will have a performance penalty. Would there be interest in an additional override to `putImageData` so that feeding the A channel could be made optional? The C++ source already loops over the image and does a skip here: https://github.com/Automattic/node-canvas/blob/v2.6.1/src/CanvasRenderingContext2d.cc#L889)
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.