Icinga / Icinga/icinga2

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

Open
#9,799 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area/api bug
Dominant language
C++
Stars
2.2k
Forks
616
Avg merge
5d 6h
Merged PRs (30d)
10

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing /v1/actions/execute-command in lib/icinga/apiactions.cpp, then follow CheckCommand::Execute() in lib/icinga/checkcommand.cpp and PluginCheckTask::ScriptFunc() in lib/methods/pluginchecktask.cpp. Reproduce the action and inspect whether the existing last check result changes; done means confirming or disproving the suspected mutation and documenting the result or adding a regression test if the project’s tests support it.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.