sml icon indicating copy to clipboard operation
sml copied to clipboard

Use-after-free after copying state mahcine with dependencies

Open ukreator opened this issue 3 years ago • 0 comments

Expected Behavior

I expect this program to run without crashing

int main(int argc, char** argv) {
    struct e1 {};
    struct dependency {
        int i = 0;
    };

    struct dependencies {
        auto operator()() const noexcept {
            using namespace boost::sml;

            const auto action = [](const dependency& d,
                                   const auto& event) {
                std::cout << d.i << std::endl;  //<------- reads from already freed memory
            };

            return make_transition_table(*"idle"_s + event<e1> / action = "s1"_s);
        }
    };

    using namespace boost::sml;
    std::vector<boost::sml::sm<dependencies>> xx;
    {
        dependency d;
        sm<dependencies> sm{d};
        xx.push_back(sm);
    }

    assert(xx[0].is("idle"_s));

    xx[0].process_event(e1{});
    assert(xx[0].is("s1"_s));
}

Actual Behavior

Reports stack-use-after-scope if running under Address sanitizer.

Steps to Reproduce the Problem

  1. build with -fsanitize=address
  2. run

Specifications

  • Version: 1.1.4
  • Platform: macos

Looks like the dependencies are captured exactly as they are specified in the actions/guards (by const ref in the example above). This makes it very easy to introduce a bug that reveals itself when everything is deployed in production. I did experiments with a more complicated SM with multiple actions capturing the same dependency and I had to capture by value in all the cases to prevent invalid memory access issues. It's very easy to overlook or introduce later (e.g. someone adds a new action lambda).

I don't know what's the right solution in this case. Maybe SML should make a copy of every dependency? Or the copy/move constructors should be disabled? There should be a mention of this in the docs somewhere (tips and tricks? important notes?)

ukreator avatar May 24 '21 01:05 ukreator