Skip to content

Commit caeaac4

Browse files
committed
fix: memory leak in XSLT transform
When a param contains a NULL byte, the exception raised by StringValueCStr prevented cleanup of the allocated array. Now we use rb_protect to free that memory before re-raising. ref: GHSA-v2fc-qm4h-8hqv (cherry picked from commit b81d745)
1 parent 25220bf commit caeaac4

2 files changed

Lines changed: 57 additions & 5 deletions

File tree

ext/nokogiri/xslt_stylesheet.c

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,35 @@ rb_xslt_stylesheet_serialize(VALUE self, VALUE xmlobj)
133133
return rval ;
134134
}
135135

136+
137+
/*
138+
* Build the C-string params array passed to xsltApplyStylesheet.
139+
*
140+
* Note: params[j] is a raw pointer into a Ruby string's buffer, and we do not pin the underlying
141+
* VALUEs against GC compaction. This is safe (despite not pinning the VALUEs) because libxslt fully
142+
* processes params (interning names, evaluating values) before template execution begins, and Ruby
143+
* callbacks can only run during template execution. By the time GC compaction is reachable, libxslt
144+
* no longer reads params[].
145+
*/
146+
typedef struct {
147+
VALUE rb_param;
148+
long param_len;
149+
const char **params;
150+
} build_xslt_params_args_t;
151+
152+
static VALUE
153+
build_xslt_params(VALUE args_ptr)
154+
{
155+
build_xslt_params_args_t *args = (build_xslt_params_args_t *)args_ptr;
156+
157+
for (long j = 0; j < args->param_len; j++) {
158+
VALUE entry = rb_ary_entry(args->rb_param, j);
159+
args->params[j] = StringValueCStr(entry);
160+
}
161+
162+
return Qnil;
163+
}
164+
136165
/*
137166
* call-seq:
138167
* transform(document)
@@ -254,7 +283,7 @@ rb_xslt_stylesheet_transform(int argc, VALUE *argv, VALUE self)
254283
xmlDocPtr c_result_document ;
255284
nokogiriXsltStylesheetTuple *wrapper;
256285
const char **params ;
257-
long param_len, j ;
286+
long param_len ;
258287
int parse_error_occurred ;
259288
int defensive_copy_p = 0;
260289

@@ -277,10 +306,17 @@ rb_xslt_stylesheet_transform(int argc, VALUE *argv, VALUE self)
277306

278307
param_len = RARRAY_LEN(rb_param);
279308
params = ruby_xcalloc((size_t)param_len + 1, sizeof(char *));
280-
for (j = 0 ; j < param_len ; j++) {
281-
VALUE entry = rb_ary_entry(rb_param, j);
282-
const char *ptr = StringValueCStr(entry);
283-
params[j] = ptr;
309+
{
310+
// populate params under rb_protect so that a raise from StringValueCStr
311+
// (e.g. on a null byte) does not leak the params allocation.
312+
build_xslt_params_args_t args = { rb_param, param_len, params };
313+
int state = 0;
314+
315+
rb_protect(build_xslt_params, (VALUE)&args, &state);
316+
if (state) {
317+
ruby_xfree(params);
318+
rb_jump_tag(state);
319+
}
284320
}
285321
params[param_len] = 0 ;
286322

test/test_memory_usage.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,5 +337,21 @@ def start_element(name, attrs = [])
337337
assert_equal(472, parser.document.data.length)
338338
end
339339
end
340+
341+
it "XSLT#transform doesn't leak on null byte in params" do
342+
# https://github.com/sparklemotion/nokogiri/security/advisories/GHSA-v2fc-qm4h-8hqv
343+
xsl = Nokogiri::XSLT.parse(<<~XSL)
344+
<?xml version="1.0"?>
345+
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
346+
<xsl:param name="p"/>
347+
<xsl:template match="/"><r><xsl:value-of select="$p"/></r></xsl:template>
348+
</xsl:stylesheet>
349+
XSL
350+
doc = Nokogiri::XML("<root/>")
351+
352+
memwatch(__method__) do
353+
assert_raises(ArgumentError) { xsl.transform(doc, ["p", "val\x00ue"]) }
354+
end
355+
end
340356
end if ENV["NOKOGIRI_MEMORY_SUITE"] && Nokogiri.uses_libxml?
341357
end

0 commit comments

Comments
 (0)