uplink(fix): Fix use-after-free bug in edge::Gateway

[GodTamIt]

Previously, from_ffi_credentials_result() would free the underlying memory of the strings stored in Gateway (the FFI EdgeCredentialsResult) upon success, resulting in a use-after-free error for anyone trying to access those strings.

Unfortunately, this will be a breaking change.

[GodTamIt]

fyi @ifraixedes - this is potentially a security vulnerability.

[ifraixedes]

@GodTamIt good catch!

I detected these issues when I wrote one of the first integration tests. When I realized the problem, I changed all of them to types that Rust owns rather than dragging references to C pointers.

I changed all of those structs that I found however, I missed some that I changed when later I realized (e.g. 9bb1cfb).

I missed this one. One of the reasons is that there is no integration test for it https://github.com/storj-thirdparty/uplink-rust/blob/main/uplink/tests/edge_test.rs#L8

Thank you very much for your contribution.

[GodTamIt]

@ifraixedes yep, all done!

Thanks for the quick reviews :)

Aside: for some of the memory freeing code paths, what's your opinion on using something like scopeguard so that memory always gets cleaned up (RAII style)?

[ifraixedes]

Aside: for some of the memory freeing code paths, what's your opinion on using something like scopeguard so that memory always gets cleaned up (RAII style)?

I got a suggestion when I talked about building this in a meetup. They said:

Because you cannot implement the Drop trait for the types generated by bindgen, you could implement a wrapper type for each of them and implement the Drop trait.

I stated it as a good idea. Unfortunately, I didn't think about it when I implemented it 😬

Did I respond to your question?

[GodTamIt]

Aside: for some of the memory freeing code paths, what's your opinion on using something like scopeguard so that memory always gets cleaned up (RAII style)?

I got a suggestion when I talked about building this in a meetup. They said:

Because you cannot implement the Drop trait for the types generated by bindgen, you could implement a wrapper type for each of them and implement the Drop trait.

I stated it as a good idea. Unfortunately, I didn't think about it when I implemented it 😬

Did I respond to your question?

Yeah that does. If I have some time, I could explore doing something like that. Otherwise, the library is working as intended so not critical. Just helps prevent leaks and bugs :)

[ifraixedes]

@GodTamIt the build fails because of unrelated issues. I'm addressing them

[ifraixedes]

You'll need to rebase once I merge #82
I'm waiting for reviews.

[ifraixedes]

@GodTamIt it's merged.

[GodTamIt]

@ifraixedes it's rebased

[ifraixedes]

@GodTamIt I will make a new release later today or tomorrow.
Thank you for your contribution.