Skip to content

Commit

Permalink
Ensure strings are zero terminated, where C-str pointers are expected.
Browse files Browse the repository at this point in the history 
Ruby-2.2 changes the String handling in such a way, that strings can
no longer be expected to be zero terminated. When zero terminated
strings are expected in the C code, StringValueCStr() must be used.
See: https://github.com/ruby/ruby/blob/v2_2_0/NEWS#L319

Using StringValuePtr() without a length check, can also become a
security issue, because it can leak the memory after the string.

This was already an issue in the pg.gem: ged/ruby-pg#5
  • Loading branch information
@larskanis
larskanis committed Nov 23, 2015
1 parent ec686ab commit f346458
Show file tree
Hide file tree
Showing 20 changed files with 91 additions and 91 deletions.
12 changes: 6 additions & 6 deletions ext/nokogiri/html_document.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
external_id = rb_ary_entry(rest, (long)1);

doc = htmlNewDoc(
RTEST(uri) ? (const xmlChar *)StringValuePtr(uri) : NULL,
RTEST(external_id) ? (const xmlChar *)StringValuePtr(external_id) : NULL
RTEST(uri) ? (const xmlChar *)StringValueCStr(uri) : NULL,
RTEST(external_id) ? (const xmlChar *)StringValueCStr(external_id) : NULL
);
rb_doc = Nokogiri_wrap_xml_document(klass, doc);
rb_obj_call_init(rb_doc, argc, argv);
Expand All @@ -39,8 +39,8 @@ static VALUE read_io( VALUE klass,
VALUE encoding,
VALUE options )
{
const char * c_url = NIL_P(url) ? NULL : StringValuePtr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValuePtr(encoding);
const char * c_url = NIL_P(url) ? NULL : StringValueCStr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValueCStr(encoding);
VALUE error_list = rb_ary_new();
VALUE document;
htmlDocPtr doc;
Expand Down Expand Up @@ -103,8 +103,8 @@ static VALUE read_memory( VALUE klass,
VALUE options )
{
const char * c_buffer = StringValuePtr(string);
const char * c_url = NIL_P(url) ? NULL : StringValuePtr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValuePtr(encoding);
const char * c_url = NIL_P(url) ? NULL : StringValueCStr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValueCStr(encoding);
int len = (int)RSTRING_LEN(string);
VALUE error_list = rb_ary_new();
VALUE document;
Expand Down
2 changes: 1 addition & 1 deletion ext/nokogiri/html_element_description.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ static VALUE name(VALUE self)
static VALUE get_description(VALUE klass, VALUE tag_name)
{
const htmlElemDesc * description = htmlTagLookup(
(const xmlChar *)StringValuePtr(tag_name)
(const xmlChar *)StringValueCStr(tag_name)
);

if(NULL == description) return Qnil;
Expand Down
2 changes: 1 addition & 1 deletion ext/nokogiri/html_entity_lookup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
static VALUE get(VALUE self, VALUE key)
{
const htmlEntityDesc * desc =
htmlEntityLookup((const xmlChar *)StringValuePtr(key));
htmlEntityLookup((const xmlChar *)StringValueCStr(key));
VALUE klass, args[3];

if(NULL == desc) return Qnil;
Expand Down
8 changes: 4 additions & 4 deletions ext/nokogiri/html_sax_parser_context.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ parse_memory(VALUE klass, VALUE data, VALUE encoding)
}

if (RTEST(encoding)) {
xmlCharEncodingHandlerPtr enc = xmlFindCharEncodingHandler(StringValuePtr(encoding));
xmlCharEncodingHandlerPtr enc = xmlFindCharEncodingHandler(StringValueCStr(encoding));
if (enc != NULL) {
xmlSwitchToEncoding(ctxt, enc);
if (ctxt->errNo == XML_ERR_UNSUPPORTED_ENCODING) {
rb_raise(rb_eRuntimeError, "Unsupported encoding %s",
StringValuePtr(encoding));
StringValueCStr(encoding));
}
}
}
Expand All @@ -47,8 +47,8 @@ parse_memory(VALUE klass, VALUE data, VALUE encoding)
static VALUE parse_file(VALUE klass, VALUE filename, VALUE encoding)
{
htmlParserCtxtPtr ctxt = htmlCreateFileParserCtxt(
StringValuePtr(filename),
StringValuePtr(encoding)
StringValueCStr(filename),
StringValueCStr(encoding)
);
return Data_Wrap_Struct(klass, NULL, deallocate, ctxt);
}
Expand Down
4 changes: 2 additions & 2 deletions ext/nokogiri/html_sax_push_parser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ static VALUE initialize_native(VALUE self, VALUE _xml_sax, VALUE _filename,

Data_Get_Struct(_xml_sax, xmlSAXHandler, sax);

if(_filename != Qnil) filename = StringValuePtr(_filename);
if(_filename != Qnil) filename = StringValueCStr(_filename);

if (!NIL_P(encoding)) {
enc = xmlParseCharEncoding(StringValuePtr(encoding));
enc = xmlParseCharEncoding(StringValueCStr(encoding));
if (enc == XML_CHAR_ENCODING_ERROR)
rb_raise(rb_eArgError, "Unsupported Encoding");
}
Expand Down
4 changes: 2 additions & 2 deletions ext/nokogiri/xml_attr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ static VALUE set_value(VALUE self, VALUE content)
xmlNode *tmp;

/* Encode our content */
buffer = xmlEncodeEntitiesReentrant(attr->doc, (unsigned char *)StringValuePtr(content));
buffer = xmlEncodeEntitiesReentrant(attr->doc, (unsigned char *)StringValueCStr(content));

attr->children = xmlStringGetNodeList(attr->doc, buffer);
attr->last = NULL;
Expand Down Expand Up @@ -61,7 +61,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)

node = xmlNewDocProp(
xml_doc,
(const xmlChar *)StringValuePtr(name),
(const xmlChar *)StringValueCStr(name),
NULL
);

Expand Down
2 changes: 1 addition & 1 deletion ext/nokogiri/xml_comment.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)

node = xmlNewDocComment(
xml_doc,
(const xmlChar *)StringValuePtr(content)
(const xmlChar *)StringValueCStr(content)
);

rb_node = Nokogiri_wrap_xml_node(klass, node);
Expand Down
36 changes: 18 additions & 18 deletions ext/nokogiri/xml_document.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ static VALUE set_encoding(VALUE self, VALUE encoding)
if (doc->encoding)
free((char *) doc->encoding); /* this may produce a gcc cast warning */

doc->encoding = xmlStrdup((xmlChar *)StringValuePtr(encoding));
doc->encoding = xmlStrdup((xmlChar *)StringValueCStr(encoding));

return encoding;
}
Expand Down Expand Up @@ -228,8 +228,8 @@ static VALUE read_io( VALUE klass,
VALUE encoding,
VALUE options )
{
const char * c_url = NIL_P(url) ? NULL : StringValuePtr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValuePtr(encoding);
const char * c_url = NIL_P(url) ? NULL : StringValueCStr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValueCStr(encoding);
VALUE error_list = rb_ary_new();
VALUE document;
xmlDocPtr doc;
Expand Down Expand Up @@ -279,8 +279,8 @@ static VALUE read_memory( VALUE klass,
VALUE options )
{
const char * c_buffer = StringValuePtr(string);
const char * c_url = NIL_P(url) ? NULL : StringValuePtr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValuePtr(encoding);
const char * c_url = NIL_P(url) ? NULL : StringValueCStr(url);
const char * c_enc = NIL_P(encoding) ? NULL : StringValueCStr(encoding);
int len = (int)RSTRING_LEN(string);
VALUE error_list = rb_ary_new();
VALUE document;
Expand Down Expand Up @@ -357,7 +357,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
version = rb_ary_entry(rest, (long)0);
if (NIL_P(version)) version = rb_str_new2("1.0");

doc = xmlNewDoc((xmlChar *)StringValuePtr(version));
doc = xmlNewDoc((xmlChar *)StringValueCStr(version));
rb_doc = Nokogiri_wrap_xml_document(klass, doc);
rb_obj_call_init(rb_doc, argc, argv);
return rb_doc ;
Expand Down Expand Up @@ -385,13 +385,13 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
* </bicycle>
* </root>
* EOXML
*
*
* doc.xpath("//tire").to_s # => ""
* doc.xpath("//part:tire", "part" => "http://general-motors.com/").to_s # => "<part:tire>Michelin Model XGV</part:tire>"
* doc.xpath("//part:tire", "part" => "http://schwinn.com/").to_s # => "<part:tire>I'm a bicycle tire!</part:tire>"
*
*
* doc.remove_namespaces!
*
*
* doc.xpath("//tire").to_s # => "<tire>Michelin Model XGV</tire><tire>I'm a bicycle tire!</tire>"
* doc.xpath("//part:tire", "part" => "http://general-motors.com/").to_s # => ""
* doc.xpath("//part:tire", "part" => "http://schwinn.com/").to_s # => ""
Expand Down Expand Up @@ -438,11 +438,11 @@ static VALUE create_entity(int argc, VALUE *argv, VALUE self)
xmlResetLastError();
ptr = xmlAddDocEntity(
doc,
(xmlChar *)(NIL_P(name) ? NULL : StringValuePtr(name)),
(xmlChar *)(NIL_P(name) ? NULL : StringValueCStr(name)),
(int) (NIL_P(type) ? XML_INTERNAL_GENERAL_ENTITY : NUM2INT(type)),
(xmlChar *)(NIL_P(external_id) ? NULL : StringValuePtr(external_id)),
(xmlChar *)(NIL_P(system_id) ? NULL : StringValuePtr(system_id)),
(xmlChar *)(NIL_P(content) ? NULL : StringValuePtr(content))
(xmlChar *)(NIL_P(external_id) ? NULL : StringValueCStr(external_id)),
(xmlChar *)(NIL_P(system_id) ? NULL : StringValueCStr(system_id)),
(xmlChar *)(NIL_P(content) ? NULL : StringValueCStr(content))
);

if(NULL == ptr) {
Expand Down Expand Up @@ -486,9 +486,9 @@ static int block_caller(void * ctx, xmlNodePtr _node, xmlNodePtr _parent)
* doc.canonicalize { |obj, parent| ... }
*
* Canonicalize a document and return the results. Takes an optional block
* that takes two parameters: the +obj+ and that node's +parent+.
* that takes two parameters: the +obj+ and that node's +parent+.
* The +obj+ will be either a Nokogiri::XML::Node, or a Nokogiri::XML::Namespace
* The block must return a non-nil, non-false value if the +obj+ passed in
* The block must return a non-nil, non-false value if the +obj+ passed in
* should be included in the canonicalized document.
*/
static VALUE canonicalize(int argc, VALUE* argv, VALUE self)
Expand Down Expand Up @@ -533,14 +533,14 @@ static VALUE canonicalize(int argc, VALUE* argv, VALUE self)
ns = calloc((size_t)ns_len+1, sizeof(xmlChar *));
for (i = 0 ; i < ns_len ; i++) {
VALUE entry = rb_ary_entry(incl_ns, i);
const char * ptr = StringValuePtr(entry);
const char * ptr = StringValueCStr(entry);
ns[i] = (xmlChar*) ptr;
}
}


xmlC14NExecute(doc, cb, ctx,
(int) (NIL_P(mode) ? 0 : NUM2INT(mode)),
xmlC14NExecute(doc, cb, ctx,
(int) (NIL_P(mode) ? 0 : NUM2INT(mode)),
ns,
(int) RTEST(with_comments),
buf);
Expand Down
6 changes: 3 additions & 3 deletions ext/nokogiri/xml_encoding_handler.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ static VALUE get(VALUE klass, VALUE key)
{
xmlCharEncodingHandlerPtr handler;

handler = xmlFindCharEncodingHandler(StringValuePtr(key));
handler = xmlFindCharEncodingHandler(StringValueCStr(key));
if(handler)
return Data_Wrap_Struct(klass, NULL, NULL, handler);

Expand All @@ -23,7 +23,7 @@ static VALUE get(VALUE klass, VALUE key)
*/
static VALUE delete(VALUE klass, VALUE name)
{
if(xmlDelEncodingAlias(StringValuePtr(name))) return Qnil;
if(xmlDelEncodingAlias(StringValueCStr(name))) return Qnil;

return Qtrue;
}
Expand All @@ -35,7 +35,7 @@ static VALUE delete(VALUE klass, VALUE name)
*/
static VALUE alias(VALUE klass, VALUE from, VALUE to)
{
xmlAddEncodingAlias(StringValuePtr(from), StringValuePtr(to));
xmlAddEncodingAlias(StringValueCStr(from), StringValueCStr(to));

return to;
}
Expand Down
2 changes: 1 addition & 1 deletion ext/nokogiri/xml_entity_reference.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)

node = xmlNewReference(
xml_doc,
(const xmlChar *)StringValuePtr(name)
(const xmlChar *)StringValueCStr(name)
);

nokogiri_root_node(node);
Expand Down
Loading

0 comments on commit f346458

Please sign in to comment.