scap_platform_init and scap_open api leads to memory corruptions

[albe19029]

We have faced memory corruption while switching form version 6.0.1+driver to 7.0.0+driver. The reason is not very clear API for new scap_platform_init method.

Before this update there was only one method scap_open, with next signature.

scap_t* scap_open(scap_open_args* oargs, char *error, int32_t *rc)

And we passed

char error[SCAP_LASTERR_SIZE] = { 0 };

from the stack without any problems.

scap_platform_init has a bit the same signature:

int32_t scap_platform_init(struct scap_platform *platform, char *lasterr, struct scap_engine_handle engine, struct scap_open_args *oargs);

But for some reason, internally scap_linux_init_platform method assign lasterr pointer:

linux_platform->m_lasterr = lasterr;

And when we log to this buffer (in our case on stack) - this leads to corruption.

I think that api is not clear. Both method should work the same. For scap_t memory is preallocated in structure.

char m_lasterr[SCAP_LASTERR_SIZE];

Why the code is working in another way for struct scap_linux_platform and don't create this buffer in structure.

[albe19029]

Or a least document that scap_platform_init has action like this, and buffer must be available till platform will be released.

[FedeDP]

Hi! Thanks for opening this issue! Yes the low level libscap library is not really thought to be used in the wild; most of the time, people are using libsinsp instead. Also pinging @gnosek that is the main author of the platform API :)

[gnosek]

the low level libscap library is not really thought to be used in the wild

Exactly :) This goes double for scap_platform :)

Or a least document that scap_platform_init has action like this, and buffer must be available till platform will be released.

I think this is the best solution. As long as we have all the moving pieces inside libscap (platform, engine, and the scap_t wrapper itself), we don't want to have distinct error buffers between them (this leads either to copying the error messages back & forth, or to dropping them when we forget to copy them). So, scap_platform shares its buffer with the caller (which is usually scap_init that has its own allocated buffer in scap_t).

Please note that the stack-allocated buffer you're passing to scap_open was used only when scap_open failed and there was no handle to store the error in (we now have allocation and initialization separated and IIRC when scap_init fails, it logs into the handle's buffer).

Both method should work the same.

I respectfully disagree. The two APIs are on two different layers of abstraction (scap_t abstracts over the engine and the platform, and honestly does little useful work).

BTW @albe19029 is your project open source? Can you share a link if so?

[albe19029]

Good day @gnosek Our project is not open source, so I can't share any link with you. Hope it is not a problem as GPL2.txt allows to use it without changes, that is why I usually create request there to make modifications in you repo.

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[poiana]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh with /remove-lifecycle rotten.

Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle rotten