Memleak and memory access error in sp_repr_css_merge_from_decl

[Kris]

This report is a follow up from Bug #1043571.

There is a memory leak in the function sp_repr_css_merge_from_decl. Moreover, because of not checking the string length of the attribute string, there illegal memory access is quite possible (reading bytes outside of the string memory). This is the original report from comment 8 issue 1 of Bug #1043571:

----------
there is a piece of code that does not check that l>2 before using strncmp, which spammed the valgrind output
with 40 something invalid reads. It should be:

    int l = strlen(value_unquoted);
    if (l>2 &&
            (!strncmp(&value_unquoted[l-2], "em", 2) ||
             !strncmp(&value_unquoted[l-2], "ex", 2)
            )) {
        units = g_strndup(&value_unquoted[l-2], 2);
        value_unquoted = g_strndup(value_unquoted, l-2);
    }

Also, please have mercy and don't use "l" for a variable, it is nearly indistinguishable from "1", especially if your eyes are as
bad as mine.
----------

[Kris (kris-degussem)]

Patch that should fix the described memory issues ...

[Kris (kris-degussem)]

share_string / SimpleNode::setAttribute leaks tracked in bug #1043571

[Kris (kris-degussem)]

Committed in revision 11771.

[David Mathog (mathog)]

The "ex" and "em" logic in the patch, and in the code before the patch, assumes that these are always lower case. Is that guaranteed to always be the case?

[Kris (kris-degussem)]

I do not know, although I would guess so. If you really want to be sure, one can modify line 366 from:
   units = value_unquoted2.substr(le-2, 2);
into something like (not tested):
   units = value_unquoted2.substr(le-2, 2).lowercase();

Note that your variant of the length fix of comment 81 of the omibus patch (Bug #988601) still leaves a memory leak. In general it is best to use c++ string classes (Glib::ustring for unicode strings or alternatively std::string for strings where you are sure you never need unicode characters) which are more flexible and prevent memory leaks associated to string allocation and deallocation.

[suresh (suresh-meetsme)]

Hello Suv,

I just downloaded 0.84.4 and installed in Ubuntu 12.04. What I observe is really surprise.

I just enabled debugging in sp-object.cpp file in every function. Now I draw spiral on screen and it shows me the path sequence in which Object passes.

I just delete the spiral and undo it again, after that I repeatedly did undo-redo, and the result was shocking. Inkscape constantly increase memory with only one Object being create and destroy.

Here are the logs.
sp-object.cpp:195 sp_object_init
sp-object.cpp(197) sp_object_init:id=abc4020, typename=SPObject
sp-object.cpp(848) sp_object_invoke_build:sp_object_invoke_build id=abc4020, typename=SPSpiral
sp-object.cpp(827) sp_object_build:sp_object_build id=abc4020, typename=SPSpiral
Modified SPRoot:svg2 0 0 2
Modified SPGroup:layer1 0 0 3
Modified SPSpiral:path10 4 0 9
sp-object.cpp(789) sp_object_remove_child:sp_object_remove_child id=9e53008, typename=SPGroup
sp-object.cpp(773) sp_object_release:sp_object_release id=abc4020, typename=SPSpiral

sp-object.cpp:227 sp_object_finalize
Modified SPRoot:svg2 0 0 2
Modified SPGroup:layer1 0 0 1

sp-object.cpp:195 sp_object_init
sp-object.cpp(197) sp_object_init:id=abc4178, typename=SPObject
sp-object.cpp(848) sp_object_invoke_build:sp_object_invoke_build id=abc4178, typename=SPSpiral
sp-object.cpp(827) sp_object_build:sp_object_build id=abc4178, typename=SPSpiral
Modified SPRoot:svg2 0 0 2
Modified SPGroup:layer1 0 0 3
Modified SPSpiral:path10 4 0 9
sp-object.cpp(789) sp_object_remove_child:sp_object_remove_child id=9e53008, typename=SPGroup
sp-object.cpp(773) sp_object_release:sp_object_release id=abc4178, typename=SPSpiral

sp-object.cpp:227 sp_object_finalize
Modified SPRoot:svg2 0 0 2
Modified SPGroup:layer1 0 0 1

This is just the debugging message I got after enable debug flag in the file. If I do that same thing repeatedly memory constantly increase.

Please look into this issue. Also It will be great If you can point me on some files which you think that might be culprit for the memory leak, So that I can look into the files and gives you better feedback or perhaps try to solve the issue.

Thanks.