arrow BinaryArray advisory

[Shnatsel]

Add an advisory for apache/arrow-rs#772 and apache/arrow-rs#773

@jorgecarleitao I'd appreciate if you could take a look.

[Shnatsel]

I believe BinaryArray is mutable, so I've adjusted the wording to include writes and not just reads.

[jorgecarleitao]

I believe BinaryArray is mutable, so I've adjusted the wording to include writes and not just reads.

How have you reached this? I think it does not expose any mutability. If it does, we have a data race problem. Arrays in arrow are expected to be immutable (under the indirection: BinaryArray -> ArrayData -> Arc<Buffer> -> Bytes(*const u8)).

[alamb]

@Shnatsel -- I wonder how did you decide to include this specific issue (and not, for example, the other items that are in the repo https://github.com/apache/arrow-rs/labels/security)?

[Shnatsel]

@alamb I simply didn't get around to the other items yet. PRs are welcome.

[alamb]

@alamb I simply didn't get around to the other items yet. PRs are welcome.

I think a heads up to the arrow developer community would be appropriate before flagging these as vulnerabilities, to be honest. I'll write an email

[Shnatsel]

That's fair. I usually post a comment on the relevant issue when a RustSec advisory is filed; I have unfortunately missed it this time.

That said, I believe the Arrow maintainers were at least aware of the issues. The issue reports demonstrating buffer overflows in safe code have been up on Github for over 2 weeks now, so creation of security advisories for them should not come as a surprise.

[alamb]

https://lists.apache.org/thread.html/r3f12f3352ca36264622d4103fcb6c7c71544dcaf0f0a7e842f00c3a0%40%3Cdev.arrow.apache.org%3E

[tarcieri]

Responding to this comment from that mailing list in particular:

The criteria used for adding the arrow issues to the RUSTSEC database is not clear to me

This vulnerability appears to fall under CWE-125: Out-of-bounds Read.

That particular vulnerability class is #3 in the 2021 CWE Top 25 Most Dangerous Software Weaknesses.

[alamb]

I guess I was mostly confused about the timing of the addition :) Nothing like a bit of more public shaming to spur some action.

(disclosure I am one of the arrow-rs maintainers -- we'll get them fixed)

[alamb]

BTW I would suggest in the future doing the process proposed by @Shnatsel above and at least commenting on the ticket that is about to get a RUSTSEC entry so that the affected community has some advanced warning.

Thank you for your work in maintaining this database

[Shnatsel]

Regarding the inclusion criteria, anything that violates the soundness principle defined in the Rustonomicon is a memory safety issue and should get an advisory.

In practical terms, if there is a piece of safe Rust code fails either any of the sanitizers or miri, it violates the soundness principle.

[alamb]

I agree with the principles -- thank you for the clarifications. I do suggest improving the communication around including a project's issues in RUSTSEC to include a bit of advanced warning for project developers.

Thanks again

[Shnatsel]

I agree the communication can be improved. Would you be open to discussing this in more detail in a few days from now?

[alamb]

I agree the communication can be improved. Would you be open to discussing this in more detail in a few days from now?

I would be happy to