From befe65bc9d31e21db05a32a258f120950c9f9019 Mon Sep 17 00:00:00 2001 From: Paul Mucur Date: Fri, 22 Sep 2023 21:14:27 +0100 Subject: [PATCH] Fix leak in RE2::Set#add See https://github.com/mudge/re2/issues/104 When we raise an exception in re2_set_add, the memory used by the std::string used to store the error message is never freed. Fix this by ensuring it goes out of scope before we call rb_raise. However, we also need a copy of what is inside it to return to the user so we take a copy of its contents as a C string first. The current longest error message inside RE2 is 35 characters long so 100 characters gives us some headroom in case new releases of RE2 add longer messages. Thanks to @peterzhu2118 for both authoring ruby_memcheck and helping find the source of these leaks. --- ext/re2/re2.cc | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ext/re2/re2.cc b/ext/re2/re2.cc index 89bcba5..7257212 100644 --- a/ext/re2/re2.cc +++ b/ext/re2/re2.cc @@ -1583,13 +1583,23 @@ static VALUE re2_set_initialize(int argc, VALUE *argv, VALUE self) { static VALUE re2_set_add(VALUE self, VALUE pattern) { StringValue(pattern); re2::StringPiece regex(RSTRING_PTR(pattern), RSTRING_LEN(pattern)); - std::string err; re2_set *s; Data_Get_Struct(self, re2_set, s); - int index = s->set->Add(regex, &err); + /* To prevent the memory of the err string leaking when we call rb_raise, + * take a copy of it and let it go out of scope. + */ + char msg[100]; + int index; + + { + std::string err; + index = s->set->Add(regex, &err); + strncpy(msg, err.c_str(), sizeof(msg)); + } + if (index < 0) { - rb_raise(rb_eArgError, "str rejected by RE2::Set->Add(): %s", err.c_str()); + rb_raise(rb_eArgError, "str rejected by RE2::Set->Add(): %s", msg); } return INT2FIX(index);