From be2fcf0b4649e78aa04df87fc486f173f1b8a6c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gim=C3=A9nez=20Silva=20Germ=C3=A9n=20Alberto?= Date: Fri, 5 Jun 2026 14:12:48 -0300 Subject: [PATCH 1/4] update git ignore --- .gitignore | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index ad7431c..79b4ae6 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,9 @@ /ext/gd/*.so /ext/gd/*.bundle demo/rsn_events_bot/rsn_events.png +.aider.chat.history.md +.aider.input.history +.aider.tags.cache.v4/ +examples/jupyter-notebooks/.ipynb_checkpoints/Hist2D-Copy1-checkpoint.ipynb +examples/jupyter-notebooks/Hist2D-Copy1.ipynb + From 2d0fa32a8a33413287928a56960ff7c7ef31388d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gim=C3=A9nez=20Silva=20Germ=C3=A9n=20Alberto?= Date: Fri, 5 Jun 2026 14:25:27 -0300 Subject: [PATCH 2/4] fix security bug in GD::Image.new and bbox bug --- CHANGELOG.md | 40 +++++++++ ext/gd/image.c | 202 +++++++++++++++++++++++++-------------------- ext/gd/text.c | 2 +- ruby-libgd.gemspec | 2 +- spec/image_spec.rb | 61 ++++++++++++++ 5 files changed, 216 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8296cf5..46b935d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,44 @@ # Changelog +## v0.3.1 + +### Security + +- Hardened `GD::Image` initialization to prevent creation of partially initialized image objects. +- `GD::Image.new` now requires explicit `width` and `height` arguments. +- Added validation to reject zero or negative image dimensions. +- Added NULL-pointer guards across image operations to prevent dereferencing uninitialized image structures. +- Improved defensive programming around image cloning and copy operations. + +### Fixed + +#### `GD::Image.new` + +- `GD::Image.new` no longer returns an object with `img == NULL`. +- Calling `GD::Image.new` without dimensions now raises: + +```ruby +ArgumentError: width and height are required + +### Fixed + +#### Text Bounding Box Rendering + +- Fixed an issue where `GD::Image#text_bbox` could unexpectedly render text onto the target image while calculating text metrics. +- Bounding box calculations are now guaranteed to be side-effect free. +- Text measurement no longer modifies image pixels. +- Prevents "ghost text" artifacts appearing in generated images when only text dimensions are requested. + +Example: + +```ruby +img.text_bbox( + "text", + font: font, + size: 45, + angle: 0 +) +``` + ## v0.3.0 ### Added diff --git a/ext/gd/image.c b/ext/gd/image.c index fd069d3..d5f38e1 100644 --- a/ext/gd/image.c +++ b/ext/gd/image.c @@ -1,24 +1,42 @@ #include "ruby_gd.h" -static VALUE gd_image_initialize(int argc, VALUE *argv, VALUE self) { +static VALUE gd_image_alloc(VALUE klass) { gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - + VALUE obj = TypedData_Make_Struct(klass, gd_image_wrapper, &gd_image_type, wrap); wrap->img = NULL; + return obj; +} +/* --------------------------------------------------------- + * Initialize + * --------------------------------------------------------- */ + +static VALUE gd_image_initialize(int argc, VALUE *argv, VALUE self) { if (argc == 0) { + rb_raise(rb_eArgError, "width and height are required"); return self; } + + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + + wrap->img = NULL; + if (argc != 2) { rb_raise(rb_eArgError, "expected 0 or 2 arguments"); } int w = NUM2INT(argv[0]); int h = NUM2INT(argv[1]); + if (w <= 0 || h <= 0) rb_raise(rb_eArgError, "width and height must be positive"); wrap->img = gdImageCreateTrueColor(w,h); + + if (!wrap->img) + rb_raise(rb_eRuntimeError, "gdImageCreateTrueColor failed"); + gdImageSaveAlpha(wrap->img, 1); gdImageAlphaBlending(wrap->img, 0); @@ -27,69 +45,45 @@ static VALUE gd_image_initialize(int argc, VALUE *argv, VALUE self) { gdImageAlphaBlending(wrap->img, 1); - int t = gdTrueColorAlpha(0,0,0,127); - gdImageFilledRectangle(wrap->img, 0, 0, w-1, h-1, t); - - if (!wrap->img) - rb_raise(rb_eRuntimeError, "gdImageCreateTrueColor failed"); - return self; } -static VALUE gd_image_initialize_true_color(VALUE self, VALUE width, VALUE height) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); +/* --------------------------------------------------------- + * initialize_copy (correct way for clone/dup) + * --------------------------------------------------------- */ - if (wrap->img) { - gdImageDestroy(wrap->img); - } - - int w = NUM2INT(width); - int h = NUM2INT(height); - if (w <= 0 || h <= 0) { - rb_raise(rb_eArgError, "width and height must be positive"); - } - - wrap->img = gdImageCreateTrueColor(w, h); - if (!wrap->img) { - rb_raise(rb_eRuntimeError, "failed to create true color image"); - } +static VALUE gd_image_initialize_copy(VALUE self, VALUE orig) { + gd_image_wrapper *src; + gd_image_wrapper *dst; - // Opcional: desactivar el fondo transparente si no lo necesitas - gdImageSaveAlpha(wrap->img, 1); - gdImageAlphaBlending(wrap->img, 0); - - int t = gdTrueColorAlpha(0,0,0,127); - gdImageFilledRectangle(wrap->img, 0,0, w-1, h-1, t); + if (self == orig) return self; - return self; -} - -static VALUE gd_image_s_new_true_color(VALUE klass, VALUE width, VALUE height) { - VALUE obj = rb_class_new_instance(0, NULL, klass); - gd_image_initialize_true_color(obj, width, height); - return obj; -} + TypedData_Get_Struct(orig, gd_image_wrapper, &gd_image_type, src); + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, dst); -static VALUE gd_image_clone(VALUE self) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!src->img) + rb_raise(rb_eRuntimeError, "cannot copy: image is NULL"); - if (!wrap->img) rb_raise(rb_eRuntimeError, "cannot clone: image is NULL"); + if (dst->img) { + gdImageDestroy(dst->img); + dst->img = NULL; + } - gdImagePtr copy = gdImageClone(wrap->img); - if (!copy) rb_raise(rb_eRuntimeError, "gdImageClone failed"); + dst->img = gdImageClone(src->img); - VALUE obj = rb_class_new_instance(0, NULL, CLASS_OF(self)); - gd_image_wrapper *w2; - TypedData_Get_Struct(obj, gd_image_wrapper, &gd_image_type, w2); - w2->img = copy; + if (!dst->img) + rb_raise(rb_eRuntimeError, "gdImageClone failed"); - return obj; + return self; } +/* --------------------------------------------------------- + * Free + * --------------------------------------------------------- */ + static void gd_image_free(void *ptr) { gd_image_wrapper *wrap = (gd_image_wrapper *)ptr; + if (!wrap) return; if (wrap->img) { @@ -104,17 +98,14 @@ static size_t gd_image_memsize(const void *ptr) { const rb_data_type_t gd_image_type = { "GD::Image", - { 0, gd_image_free, gd_image_memsize, 0 }, // <-- dcompact agregado + { 0, gd_image_free, gd_image_memsize, }, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; -static VALUE gd_image_alloc(VALUE klass) { - gd_image_wrapper *wrap; - VALUE obj = TypedData_Make_Struct(klass, gd_image_wrapper, &gd_image_type, wrap); - wrap->img = NULL; - return obj; -} +/* --------------------------------------------------------- + * Image loading + * --------------------------------------------------------- */ static VALUE gd_image_open(VALUE klass, VALUE path) { const char *filename = StringValueCStr(path); @@ -141,44 +132,44 @@ static VALUE gd_image_open(VALUE klass, VALUE path) { } fclose(f); - if (!img) rb_raise(rb_eRuntimeError, "image decode failed"); - VALUE obj = rb_class_new_instance(0, NULL, klass); + if (!img) + rb_raise(rb_eRuntimeError, "image decode failed"); + + VALUE obj = rb_obj_alloc(klass); + gd_image_wrapper *wrap; TypedData_Get_Struct(obj, gd_image_wrapper, &gd_image_type, wrap); + wrap->img = img; return obj; } -static VALUE gd_image_alpha_blending(VALUE self, VALUE flag) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - - gdImageAlphaBlending(wrap->img, RTEST(flag)); - return self; -} - -static VALUE gd_image_save_alpha(VALUE self, VALUE flag) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - - gdImageSaveAlpha(wrap->img, RTEST(flag)); - return self; -} +/* --------------------------------------------------------- + * Image info + * --------------------------------------------------------- */ static VALUE gd_image_width(VALUE self) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - return INT2NUM(gdImageSX(wrap->img)); + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + return INT2NUM(gdImageSX(wrap->img)); } static VALUE gd_image_height(VALUE self) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - return INT2NUM(gdImageSY(wrap->img)); + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + return INT2NUM(gdImageSY(wrap->img)); } +/* --------------------------------------------------------- + * Copy operations + * --------------------------------------------------------- */ + static VALUE gd_image_copy ( VALUE self, VALUE src, VALUE dx, VALUE dy, @@ -191,6 +182,9 @@ static VALUE gd_image_copy ( TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, dst); TypedData_Get_Struct(src, gd_image_wrapper, &gd_image_type, srcw); + if (!dst->img) rb_raise(rb_eRuntimeError, "image not initialized"); + if (!srcw->img) rb_raise(rb_eRuntimeError, "source image not initialized"); + gdImageCopy( dst->img, srcw->img, @@ -216,6 +210,9 @@ static VALUE gd_image_copy_resize(int argc, VALUE *argv, VALUE self) { TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, dst); TypedData_Get_Struct(src_img, gd_image_wrapper, &gd_image_type, src); + if (!dst->img) rb_raise(rb_eRuntimeError, "image not initialized"); + if (!src->img) rb_raise(rb_eRuntimeError, "source image not initialized"); + int dx = NUM2INT(dst_x); int dy = NUM2INT(dst_y); int sx = NUM2INT(src_x); @@ -225,9 +222,7 @@ static VALUE gd_image_copy_resize(int argc, VALUE *argv, VALUE self) { int dw = NUM2INT(dst_w); int dh = NUM2INT(dst_h); - int do_resample = RTEST(resample); - - if (do_resample) { + if (RTEST(resample)) { gdImageCopyResampled( dst->img, src->img, dx, dy, @@ -248,12 +243,34 @@ static VALUE gd_image_copy_resize(int argc, VALUE *argv, VALUE self) { return self; } +/* --------------------------------------------------------- + * Flags + * --------------------------------------------------------- */ + +static VALUE gd_image_alpha_blending(VALUE self, VALUE flag) { + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + gdImageAlphaBlending(wrap->img, RTEST(flag)); + return self; +} + +static VALUE gd_image_save_alpha(VALUE self, VALUE flag) { + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + gdImageSaveAlpha(wrap->img, RTEST(flag)); + return self; +} + static VALUE gd_image_antialias(VALUE self, VALUE flag) { gd_image_wrapper *wrap; TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); if (!wrap->img) - rb_raise(rb_eRuntimeError, "image is NULL"); + rb_raise(rb_eRuntimeError, "image not initialized"); if (RTEST(flag)) { int aa = gdTrueColorAlpha(255,255,255,0); @@ -266,23 +283,30 @@ static VALUE gd_image_antialias(VALUE self, VALUE flag) { return flag; } +/* --------------------------------------------------------- + * Class definition + * --------------------------------------------------------- */ void gd_define_image(VALUE mGD) { + VALUE cGDImage = rb_define_class_under(mGD, "Image", rb_cObject); rb_define_alloc_func(cGDImage, gd_image_alloc); + rb_define_method(cGDImage, "initialize", gd_image_initialize, -1); + rb_define_method(cGDImage, "initialize_copy", gd_image_initialize_copy, 1); + rb_define_method(cGDImage, "width", gd_image_width, 0); rb_define_method(cGDImage, "height", gd_image_height, 0); - rb_define_method(cGDImage, "initialize", gd_image_initialize, -1); + rb_define_method(cGDImage, "copy_resize", gd_image_copy_resize, -1); rb_define_method(cGDImage, "copy", gd_image_copy, 7); - rb_define_method(cGDImage, "clone", gd_image_clone, 0); + rb_define_singleton_method(cGDImage, "open", gd_image_open, 1); - rb_define_singleton_method(cGDImage, "new_true_color", gd_image_s_new_true_color, 2); + rb_define_method(cGDImage, "alpha_blending=", gd_image_alpha_blending, 1); rb_define_method(cGDImage, "save_alpha=", gd_image_save_alpha, 1); + rb_define_method(cGDImage, "antialias=", gd_image_antialias, 1); rb_define_method(cGDImage, "antialias", gd_image_antialias, 1); - } diff --git a/ext/gd/text.c b/ext/gd/text.c index 238eebd..0483fdb 100644 --- a/ext/gd/text.c +++ b/ext/gd/text.c @@ -56,7 +56,7 @@ static VALUE gd_image_text_bbox(VALUE self, VALUE text, VALUE opts) { memset(&extra, 0, sizeof(extra)); char *err = gdImageStringFTEx( - wrap->img, + NULL, brect, 0, /* no color, we don't draw */ font, diff --git a/ruby-libgd.gemspec b/ruby-libgd.gemspec index 1f177cc..578a50f 100644 --- a/ruby-libgd.gemspec +++ b/ruby-libgd.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = "ruby-libgd" - s.version = "0.3.0" + s.version = "0.3.1" s.summary = "Native Ruby bindings for libgd" s.description = "High-performance native Ruby bindings to libgd for image generation, drawing, filters, alpha blending, and transformations." s.authors = ["Germán Alberto Giménez Silva"] diff --git a/spec/image_spec.rb b/spec/image_spec.rb index b162b85..59b5466 100644 --- a/spec/image_spec.rb +++ b/spec/image_spec.rb @@ -1,6 +1,67 @@ require "spec_helper" RSpec.describe GD::Image do + # ───────────────────────────────────────────────────────────── + # initialize + # ───────────────────────────────────────────────────────────── + + describe ".new" do + context "with no arguments" do + it "raises ArgumentError" do + expect { GD::Image.new }.to raise_error(ArgumentError, /width and height are required/) + end + end + + context "with a single argument" do + it "raises ArgumentError" do + expect { GD::Image.new(100) }.to raise_error(ArgumentError, /expected 0 or 2 arguments/) + end + end + + context "with valid dimensions" do + it "creates the image without error" do + expect { GD::Image.new(100, 100) }.not_to raise_error + end + end + + context "with dimensions <= 0" do + it "raises ArgumentError when width is 0" do + expect { GD::Image.new(0, 100) }.to raise_error(ArgumentError, /must be positive/) + end + + it "raises ArgumentError when height is negative" do + expect { GD::Image.new(100, -1) }.to raise_error(ArgumentError, /must be positive/) + end + end + end + + # ───────────────────────────────────────────────────────────── + # NULL guard on instance methods + # ───────────────────────────────────────────────────────────── + + describe "methods on an uninitialized image" do + let(:img) { GD::Image.allocate } # wrap->img = NULL + + it "#width raises RuntimeError" do + expect { img.width }.to raise_error(RuntimeError, /not initialized/) + end + + it "#height raises RuntimeError" do + expect { img.height }.to raise_error(RuntimeError, /not initialized/) + end + + it "#alpha_blending= raises RuntimeError" do + expect { img.alpha_blending = true }.to raise_error(RuntimeError, /not initialized/) + end + + it "#save_alpha= raises RuntimeError" do + expect { img.save_alpha = true }.to raise_error(RuntimeError, /not initialized/) + end + + it "#antialias= raises RuntimeError" do + expect { img.antialias = true }.to raise_error(RuntimeError, /not initialized/) + end + end it "creates and saves a blank image" do img = GD::Image.new(100,100) img.save("#{TMP}/blank.png") From 1246d53ed7c1126f8a0c55d1b71ea397bbc54dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gim=C3=A9nez=20Silva=20Germ=C3=A9n=20Alberto?= Date: Fri, 5 Jun 2026 15:15:52 -0300 Subject: [PATCH 3/4] update new true color --- ext/gd/image.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/ext/gd/image.c b/ext/gd/image.c index d5f38e1..f62baf6 100644 --- a/ext/gd/image.c +++ b/ext/gd/image.c @@ -48,6 +48,41 @@ static VALUE gd_image_initialize(int argc, VALUE *argv, VALUE self) { return self; } +static VALUE gd_image_initialize_true_color(VALUE self, VALUE width, VALUE height) { + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + + if (wrap->img) { + gdImageDestroy(wrap->img); + } + + int w = NUM2INT(width); + int h = NUM2INT(height); + if (w <= 0 || h <= 0) { + rb_raise(rb_eArgError, "width and height must be positive"); + } + + wrap->img = gdImageCreateTrueColor(w, h); + if (!wrap->img) { + rb_raise(rb_eRuntimeError, "failed to create true color image"); + } + + // Opcional: desactivar el fondo transparente si no lo necesitas + gdImageSaveAlpha(wrap->img, 1); + gdImageAlphaBlending(wrap->img, 0); + + int t = gdTrueColorAlpha(0,0,0,127); + gdImageFilledRectangle(wrap->img, 0,0, w-1, h-1, t); + + return self; +} + +static VALUE gd_image_s_new_true_color(VALUE klass, VALUE width, VALUE height) { + VALUE img = rb_obj_alloc(klass); + rb_funcall(img, rb_intern("initialize"), 2, width, height); + return img; +} + /* --------------------------------------------------------- * initialize_copy (correct way for clone/dup) * --------------------------------------------------------- */ @@ -303,6 +338,7 @@ void gd_define_image(VALUE mGD) { rb_define_method(cGDImage, "copy", gd_image_copy, 7); rb_define_singleton_method(cGDImage, "open", gd_image_open, 1); + rb_define_singleton_method(cGDImage, "new_true_color", gd_image_s_new_true_color, 2); rb_define_method(cGDImage, "alpha_blending=", gd_image_alpha_blending, 1); rb_define_method(cGDImage, "save_alpha=", gd_image_save_alpha, 1); From 8ab6ef1cb86f91cbf877e5078651dae03a47fba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gim=C3=A9nez=20Silva=20Germ=C3=A9n=20Alberto?= Date: Fri, 5 Jun 2026 15:27:38 -0300 Subject: [PATCH 4/4] update initialize spec --- spec/image/initialize_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/image/initialize_spec.rb b/spec/image/initialize_spec.rb index 06a8924..2796408 100644 --- a/spec/image/initialize_spec.rb +++ b/spec/image/initialize_spec.rb @@ -14,9 +14,12 @@ expect { GD::Image.new(-1, 10) }.to raise_error(ArgumentError) end - it "allows empty initialize" do + it "requires width and height" do img = GD::Image.allocate - expect { img.send(:initialize) }.not_to raise_error + + expect { + img.send(:initialize) + }.to raise_error(ArgumentError, /width and height are required/) end end end