Skip to content

Commit

Permalink
fix: add missing buffer size check in nativeImage (#17568)
Browse files Browse the repository at this point in the history
  • Loading branch information
@trop @codebytere
trop[bot] authored and codebytere committed Mar 27, 2019
1 parent 0032a52 commit f3a2c27
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 53 deletions.
130 changes: 77 additions & 53 deletions atom/common/api/atom_api_native_image.cc
View file
Expand Up @@ -81,49 +81,71 @@ float GetScaleFactorFromOptions(mate::Arguments* args) {
return scale_factor;
}

bool AddImageSkiaRep(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
int width,
int height,
double scale_factor) {
auto decoded = std::make_unique<SkBitmap>();
bool AddImageSkiaRepFromPNG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
double scale_factor) {
SkBitmap bitmap;
if (!gfx::PNGCodec::Decode(data, size, &bitmap))
return false;

image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
return true;
}

bool AddImageSkiaRepFromJPEG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
double scale_factor) {
auto bitmap = gfx::JPEGCodec::Decode(data, size);
if (!bitmap)
return false;

// `JPEGCodec::Decode()` doesn't tell `SkBitmap` instance it creates
// that all of its pixels are opaque, that's why the bitmap gets
// an alpha type `kPremul_SkAlphaType` instead of `kOpaque_SkAlphaType`.
// Let's fix it here.
// TODO(alexeykuzmin): This workaround should be removed
// when the `JPEGCodec::Decode()` code is fixed.
// See https://github.com/electron/electron/issues/11294.
bitmap->setAlphaType(SkAlphaType::kOpaque_SkAlphaType);

image->AddRepresentation(gfx::ImageSkiaRep(*bitmap, scale_factor));
return true;
}

bool AddImageSkiaRepFromBuffer(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
int width,
int height,
double scale_factor) {
// Try PNG first.
if (!gfx::PNGCodec::Decode(data, size, decoded.get())) {
// Try JPEG.
decoded = gfx::JPEGCodec::Decode(data, size);
if (decoded) {
// `JPEGCodec::Decode()` doesn't tell `SkBitmap` instance it creates
// that all of its pixels are opaque, that's why the bitmap gets
// an alpha type `kPremul_SkAlphaType` instead of `kOpaque_SkAlphaType`.
// Let's fix it here.
// TODO(alexeykuzmin): This workaround should be removed
// when the `JPEGCodec::Decode()` code is fixed.
// See https://github.com/electron/electron/issues/11294.
decoded->setAlphaType(SkAlphaType::kOpaque_SkAlphaType);
}
}
if (AddImageSkiaRepFromPNG(image, data, size, scale_factor))
return true;

if (!decoded) {
// Try Bitmap
if (width > 0 && height > 0) {
decoded.reset(new SkBitmap);
decoded->allocN32Pixels(width, height, false);
decoded->setPixels(
const_cast<void*>(reinterpret_cast<const void*>(data)));
} else {
return false;
}
}
// Try JPEG second.
if (AddImageSkiaRepFromJPEG(image, data, size, scale_factor))
return true;

if (width == 0 || height == 0)
return false;

auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType);
if (size < info.computeMinByteSize())
return false;

image->AddRepresentation(gfx::ImageSkiaRep(*decoded, scale_factor));
SkBitmap bitmap;
bitmap.allocN32Pixels(width, height, false);
bitmap.setPixels(const_cast<void*>(reinterpret_cast<const void*>(data)));

image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
return true;
}

bool AddImageSkiaRep(gfx::ImageSkia* image,
const base::FilePath& path,
double scale_factor) {
bool AddImageSkiaRepFromPath(gfx::ImageSkia* image,
const base::FilePath& path,
double scale_factor) {
std::string file_contents;
{
base::ThreadRestrictions::ScopedAllowIO allow_io;
Expand All @@ -134,7 +156,8 @@ bool AddImageSkiaRep(gfx::ImageSkia* image,
const unsigned char* data =
reinterpret_cast<const unsigned char*>(file_contents.data());
size_t size = file_contents.size();
return AddImageSkiaRep(image, data, size, 0, 0, scale_factor);

return AddImageSkiaRepFromBuffer(image, data, size, 0, 0, scale_factor);
}

bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
Expand All @@ -143,12 +166,12 @@ bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
std::string filename(path.BaseName().RemoveExtension().AsUTF8Unsafe());
if (base::MatchPattern(filename, "*@*x"))
// Don't search for other representations if the DPI has been specified.
return AddImageSkiaRep(image, path, GetScaleFactorFromPath(path));
return AddImageSkiaRepFromPath(image, path, GetScaleFactorFromPath(path));
else
succeed |= AddImageSkiaRep(image, path, 1.0f);
succeed |= AddImageSkiaRepFromPath(image, path, 1.0f);

for (const ScaleFactorPair& pair : kScaleFactorPairs)
succeed |= AddImageSkiaRep(
succeed |= AddImageSkiaRepFromPath(
image, path.InsertBeforeExtensionASCII(pair.name), pair.scale);
return succeed;
}
Expand Down Expand Up @@ -423,19 +446,20 @@ void NativeImage::AddRepresentation(const mate::Dictionary& options) {
v8::Local<v8::Value> buffer;
GURL url;
if (options.Get("buffer", &buffer) && node::Buffer::HasInstance(buffer)) {
AddImageSkiaRep(
&image_skia,
reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
node::Buffer::Length(buffer), width, height, scale_factor);
skia_rep_added = true;
auto* data = reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer));
auto size = node::Buffer::Length(buffer);
skia_rep_added = AddImageSkiaRepFromBuffer(&image_skia, data, size, width,
height, scale_factor);
} else if (options.Get("dataURL", &url)) {
std::string mime_type, charset, data;
if (net::DataURL::Parse(url, &mime_type, &charset, &data)) {
if (mime_type == "image/png" || mime_type == "image/jpeg") {
AddImageSkiaRep(&image_skia,
reinterpret_cast<const unsigned char*>(data.c_str()),
data.size(), width, height, scale_factor);
skia_rep_added = true;
auto* data_ptr = reinterpret_cast<const unsigned char*>(data.c_str());
if (mime_type == "image/png") {
skia_rep_added = AddImageSkiaRepFromPNG(&image_skia, data_ptr,
data.size(), scale_factor);
} else if (mime_type == "image/jpeg") {
skia_rep_added = AddImageSkiaRepFromJPEG(&image_skia, data_ptr,
data.size(), scale_factor);
}
}
}
Expand Down Expand Up @@ -526,9 +550,9 @@ mate::Handle<NativeImage> NativeImage::CreateFromBuffer(
}

gfx::ImageSkia image_skia;
AddImageSkiaRep(&image_skia,
reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
node::Buffer::Length(buffer), width, height, scale_factor);
AddImageSkiaRepFromBuffer(
&image_skia, reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
node::Buffer::Length(buffer), width, height, scale_factor);
return Create(args->isolate(), gfx::Image(image_skia));
}

Expand Down
17 changes: 17 additions & 0 deletions spec/api-native-image-spec.js
View file
Expand Up @@ -132,6 +132,11 @@ describe('nativeImage module', () => {
expect(nativeImage.createFromBuffer(Buffer.from([])).isEmpty())
})

it('returns an empty image when the buffer is too small', () => {
const image = nativeImage.createFromBuffer(Buffer.from([1, 2, 3, 4]), { width: 100, height: 100 })
expect(image.isEmpty()).to.be.true()
})

it('returns an image created from the given buffer', () => {
const imageA = nativeImage.createFromPath(path.join(__dirname, 'fixtures', 'assets', 'logo.png'))

Expand Down Expand Up @@ -452,6 +457,18 @@ describe('nativeImage module', () => {
})

describe('addRepresentation()', () => {
it('does not add representation when the buffer is too small', () => {
const image = nativeImage.createEmpty()

image.addRepresentation({
buffer: Buffer.from([1, 2, 3, 4]),
width: 100,
height: 100
})

expect(image.isEmpty()).to.be.true()
})

it('supports adding a buffer representation for a scale factor', () => {
const image = nativeImage.createEmpty()

Expand Down

0 comments on commit f3a2c27

Please sign in to comment.