Suspected bug: /v1/actions/execute-command may accidentally update existing check result

[julianbrost]

This is something that caught my attention while looking over something else. I haven't tested if the bug I suspect actually exists, I just wanted to write this down here so it isn't forgotten. Feel free to confirm or disprove it.

The handler for /v1/actions/execute-command takes the last check result of a checkable: https://github.com/Icinga/icinga2/blob/eddd4c7bf7554a3700abe880781a7646703b7e5f/lib/icinga/apiactions.cpp#L758

And passes it on to CheckCommand::Execute(): https://github.com/Icinga/icinga2/blob/eddd4c7bf7554a3700abe880781a7646703b7e5f/lib/icinga/apiactions.cpp#L784

Which in turn invokes the function in the execute attribute: https://github.com/Icinga/icinga2/blob/eddd4c7bf7554a3700abe880781a7646703b7e5f/lib/icinga/checkcommand.cpp#L13-L22

This can for example be PluginCheckTask::ScriptFunc() which starts a process and passes the check result to a callback function: https://github.com/Icinga/icinga2/blob/eddd4c7bf7554a3700abe880781a7646703b7e5f/lib/methods/pluginchecktask.cpp#L50-L52

Which then uses the check result like an output parameter: https://github.com/Icinga/icinga2/blob/eddd4c7bf7554a3700abe880781a7646703b7e5f/lib/methods/pluginchecktask.cpp#L80-L86

So if I haven't overlooked anything, that seems to be a bug than can corrupt the last check result in memory.

[Al2Klimov]

Yes, seems broken, especially:

https://github.com/Icinga/icinga2/blob/eddd4c7bf7554a3700abe880781a7646703b7e5f/lib/icinga/apiactions.cpp#L758-L761

🙈

doesn't quite work as usual:

https://github.com/Icinga/icinga2/blob/66088050b5a9e05fa70a2305419a9900e42713f4/lib/icinga/checkable-check.cpp#L581-L590