GNOME Bugzilla – Bug 703818
Object returned inside switch argument goes out of scope before evaluation
Last modified: 2018-05-22 14:52:26 UTC
If a function returns an object (doesn't matter if it descends from Object) and the function is called as an argument to a switch statement, the object is unref'd before the switch-case block completes. Minimal test case: class Xyzzy : Object { public string foo { get; private set; } public Xyzzy(string foo) { this.foo = foo; } } Xyzzy x(string str) { return new Xyzzy(str); } void main(string[] args) { switch (x("abc").foo) { case "abc": stdout.printf("found\n"); break; default: stdout.printf("not found\n"); break; } } x("abc").foo is "abc", but in the C code, the object is unref'd before evaluating the case blocks: _tmp0_ = x ("abc"); _tmp1_ = _tmp0_; _tmp2_ = xyzzy_get_foo (_tmp1_); _tmp3_ = _tmp2_; _tmp4_ = _tmp3_; ** _g_object_unref0 (_tmp1_); _tmp5_ = _tmp4_; ** _tmp7_ = (NULL == _tmp5_) ? 0 : g_quark_from_string (_tmp5_); if (_tmp7_ == ((0 != _tmp6_label0) ? _tmp6_label0 : (_tmp6_label0 = Lines with "**" are of interest here.
Created attachment 248653 [details] Test case
It is arguably expected as the code is equivalent to: unowned string tmp = x("abc").foo; switch (tmp) { ... } Now the problem with using the owned elements is that copying can be expensive (it contains memory barrier and is O(n)). On the other hand I don't think Vala is planning to use anything more complicated then strings ATM so computation of quark could be just shifted. (I hoped to convince Vala devs at some point in future to add tagged unions which could run into similar problems and could not used the shift of computation trick). (In reply to comment #0) > ** _tmp7_ = (NULL == _tmp5_) ? 0 : g_quark_from_string (_tmp5_); This should probably be g_quark_try_string instead of g_quark_from_string to prevent DOS attack similar to Ruby symbol attacks. In hypothetical Vala webserver the code handling HTTP request could look like: switch(method) { case "GET": .... .... } Then attacker can feed various methods names which will be converted into quarks and stored indefinitely in memory. At some point all memory will be used by quarks (or other limitations will come to play) and program will crash.
(In reply to comment #2) > It is arguably expected as the code is equivalent to: > > unowned string tmp = x("abc").foo; > switch (tmp) { > ... > } I had considered that and I can see the logic to what you're saying. I'd still argue this is a bug (or, in the least, undesirable behavior) because there's an expectation that the switch statement's argument would persist while being compared to the case labels until the switch block exits.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/393.