search

AntelopeIO / leap

C++ implementation of the Antelope protocol
Other
113 stars 69 forks source link

[5.0] avoid using a stack variable after return #2309

Closed spoonincode closed 3 months ago

spoonincode commented 3 months ago

http_max_response_time is a variable declared on that stack as part of plugin_startup() https://github.com/AntelopeIO/leap/blob/c301152420c381a08aadf1f4d1f2b59414110418/plugins/producer_api_plugin/producer_api_plugin.cpp#L94 CALL_WITH_400 captures all by reference https://github.com/AntelopeIO/leap/blob/c301152420c381a08aadf1f4d1f2b59414110418/plugins/producer_api_plugin/producer_api_plugin.cpp#L23-L26 INVOKE_R_R_D then goes on to use the reference to http_max_response_time which is stale as plugin_startup() has long since returned once HTTP requests are made https://github.com/AntelopeIO/leap/blob/c301152420c381a08aadf1f4d1f2b59414110418/plugins/producer_api_plugin/producer_api_plugin.cpp#L65-L66

This problem seemed to have been introduced in 5.0. Since this is in producer_api, which isn't expected to ever be exposed publicly, not considering this a security defect.

ericpassmore commented 3 months ago

Note:start group: CLEANCODE category: INTERNALS summary: Cleanup usage of stack variables in producer api plugin. Note: end

spoonincode commented 3 months ago

safer

Well, personally for non-trivial usages of lambdas -- such as this case -- I like to see explicit capture lists. So [&producer, &http] in this case. Does that work?

heifner commented 3 months ago

safer

Well, personally for non-trivial usages of lambdas -- such as this case -- I like to see explicit capture lists. So [&producer, &http] in this case. Does that work?

Sounds good.

spoonincode commented 3 months ago

that was probably a bad idea since now clang warns of unused lambda captures