data-control: Fix use-after-free in on_receive

[layercak3]

If a client is closed between the time that one of its receive contexts is created and the time that the receive context is finished reading from its fd, when it is finished and finally calls nvnc_send_cut_text it will access ctx->data_control->server, but data_control is part of the wayvnc_client object which was freed when the client was closed.

The only purpose of the data_control pointer being in receive_context was to pass data_control->server to nvnc_send_cut_text, so receive_context can just store that server pointer itself.

This was the issue I found in https://github.com/any1/neatvnc/pull/114#issuecomment-2303229294 Can be reproduced by having a VNC client open, then wl-copy < /some/obnoxiously/large/uncompressible/text on the server and immediately closing the VNC client after running that command. I have read and understood CONTRIBUTING.md.

[any1]

Thanks for tracking this down!

~The aml_handle is responsible for cleaning up the context, and it shouldn't call on_receive after it's cleaned up, so this looks more like an aml bug.~

[any1]

Actually, the handler should be referenced by data_control and stopped and unreffed in data_control_destroy.

[layercak3]

Actually, the handler should be referenced by data_control and stopped and unreffed in data_control_destroy.

That makes sense. There can be multiple handlers running concurrently so they would need to be stored in some list/queue though.

I also think it that it makes sense that the receive context is decoupled from the client. I thought that was the intention when reading the code, and none of the things in receive_context are necessarily tied to the client (cleaned up when it's gone).

Edit: But I guess aml_stop would then need to be called for them when the server stops. So between server and client it makes more sense to just stop them when the client is destroyed if they are still running.

[layercak3]

I just added the receive context/aml handler to a linked list so I can stop the receive contexts when closing the client. I'll properly check this and test again in some hours. I'm not familiar with aml and need to learn this but right now I think I need to aml_unref too (probably in on_receive too)

[layercak3]

I understand it better now. Thanks for your help and blog post.

[any1]

blog post?

[layercak3]

https://andri.yngvason.is/managing-object-lifetimes-in-c.html It's pretty basic stuff but I don't have much experience :p

[any1]

LGTM.

[any1]

Thanks!