[Bug] Double free in tcpedit_dlt_cleanup in tcprewrite

[iskindar]

[Bug] Double free in tcpedit_dlt_cleanup in tcprewrite

[GabrielGanne]

Hi,

I had a look and it seems that juniper has an exception in the way the plugins works with regard to the extra buffer in question: tcpreplay works with the assumption that there only ever is a single link layer plugin which is mostly true except here: Juniper has a special call to tcpedit_dlt_copy_decoder_state() which causes the ctx and subctx to share a reference to the decoded_extra buffer, and the double call through the backtrace as said in this description indeed causes the issue.

I also note that the plugin architecture is quite nice and should absolutely allow juniper to work as it does. I mean it would be a shame to break it IMHO.
Since each plugin is working with the assumption that it owns the decoded_extra buffer, I suggest to just give each one its own. That would cost a bit more, but not significantly so, and it would also enable the ability to rewrite one link layer into another.

I won't have much time to work on this next month, but I'm willing to work on this after if you want.

Also, this CVE does not seem that bad to me, but if I'm wrong I believe you could use 5ad9d1d701e644ed5b8821456e31acf2e72920c to work around this issue safely.

[bsmojver]

@fklassen, it seems that somehow freeing of sub-contexts actually frees something that gets freed again (not that I really understand the code). This particular plugin is calling these sub-context de-allocations, which then get called again.

Are sub-contexts copies of contexts and setting pointers of allocated memory to NULL in these copies isn't reflected in the originals, causing double free?

[carnil]

It looks this got CVE-2023-4256 assigned.