Add advisory for image: multiple unfixed panics and OOM in decoders

[BrianMcWilliams]

Advisory for image crate: Multiple unfixed panics and OOM in image decoders (CWE-248, CWE-400)

Crafted image files crash the process via panic!/unwrap() in BMP, PNG, TIFF, GIF, WebP decoders. References existing open issues #2672 and #2664. No patch exists.

[djc]

@197g @fintelia any thoughts on this advisory? Do you consider this in scope for the image crate (or its constituent decoder crates)?

[197g]

Sort of? I believe the advisory is not completely unwarranted. We did at some point intend to have a subset that only enables decoders for which this advisory is in scope. It is:

• in-scope for png; but I'm not aware of what they're referring to.
• in-scope for tiff (where I'm aware of panics due to jpeg that are on my priority list)
• in-scope for gif; but again not aware of any specific issue
• in-scope for WebP
• in-scope for BMP: quite recently in-scope; but more concrete cases should be listed so we can check if recent patches upstreamed as part of Chromium experimental adoption have fixed them.

@BrianMcWilliams can you expand on the connection to the two linked issues? #2672 is an unmerged feature PR (maybe typo'd)? #2664 is avif-specific and only spawns more thread than intended, it's not an OOM or panic as far as I'm aware (maybe in specific system configurations?). In the interest of being able to fill unaffected or patched fields it would be desirable to have more concrete scoping to the different decoder crates where possible.

[fintelia]

I disagree. This advisory is completely unwarranted.

The criteria for issuing an advisory specifically says Panics in code advertised as "panic-free" . The code has never been advertised as panic free. In fact, the possibility of panic on malicious input has been listed as a known issue for years.

Additionally, we do expose configurable allocation limits for image width/height, so this part is blatantly untrue:

image dimensions from file headers are trusted for buffer allocation without configurable limits

[fintelia]

Based on the other PRs filed by this account, I'm pretty convinced that this advisory is LLM slop. The previous attempted advisory against image was also LLM generated.

To me, this reflects poorly on RustSec. I think the RustSec organization should be considering much more seriously about what its role is in the ecosystem and what impact it is having.

For one thing, why isn't there bold text all over this repository telling reporters that they should first responsibly disclose vulnerabilities upstream according to their security policies? It is somewhat ridiculous that currently someone can create a publicly visible advisory, link two unrelated issues in crate's repository (one of which is actually a PR rather than an issue!), and the triage response is pinging the crate maintainers to respond further.

[djc]

To me, this reflects poorly on RustSec. I think the RustSec organization should be considering much more seriously about what its role is in the ecosystem and what impact it is having.

For one thing, why isn't there bold text all over this repository telling reporters that they should first responsibly disclose vulnerabilities upstream according to their security policies? It is somewhat ridiculous that currently someone can create a publicly visible advisory, link two unrelated issues in crate's repository (one of which is actually a PR rather than an issue!), and the triage response is pinging the crate maintainers to respond further.

For now, all RustSec maintenance is volunteer-based -- I'm the most active maintainer and was on vacation last week so I did not have a lot of spare time to look into the details. We have not had substantial slop PRs so far (that I'm aware of). I consider RustSec mostly an efficient distribution mechanism -- we obviously don't have expertise in every crate's internals and so a maintainer can much more efficiently judge advisory requests.

I did consider adding a pull request template last week when this reporter started submitting slop, but I haven't gotten to it yet (see also: vacation). It's also not obvious to me that a submitter like this will take heed from whatever we write in "bold text all over this repository".

[197g]

I agree with the fruitlessness of further guidelines. Asking for concrete details and then closing is a bit of noise; but for me not a critical level at the moment. I don't want to have a precedent of presumptively closing security issues due to a generic lack of reproduction either; especially with there being some configurations where an accurate and well-researched report might be beneficial (we have a lot of downloads on outdated crate versions that are needlessly brittle).

@djc Two points to think about: Since you had to ask for scope, is there some way we as maintainers could indicate it beforehand or in machine readable form? I've also noticed that the template does not have an [affected] features setting which would have been most appropriate here if there was a problem with one of the bindings.

[fintelia]

I looked through the docs in this repository and not only wasn't it written in bold text, but I couldn't find anywhere that said to privately report security vulnerabilities upstream before opening an advisory. In fact, there's a big red button in the README encouraging folks to do the opposite and open publicly visible PRs here right away.

Private vulnerability reporting (aka "responsible disclosure") makes the entire process of handling a possible vulnerability less rushed and less stressful, and prevents harm if the vulnerability turns out to be real. It accounts for the possibility that the recipients might be on vacation or otherwise unable to respond right away There's a reason that the security policies for tokio, hyper, wgpu, and countless other projects all say to report vulnerabilities privately.

There's also other benefits to having the initial reporting happen upstream. For one thing, the entire conversation isn't happening one click of the merge button away from having a security alert going out to potentially hundreds or thousands of people. And when discussions happen in project spaces, the project leaders can create their own issue templates, rename inaccurate titles, etc.

[alex]

I don't think any of the rustsec maintainers disagree. At most this is a matter of missing documentation (which is almost certainly missing precisely because all the rustsec maintainers take it for granted). If you'd like to contribute a pull request to improve the reporting templates, that'd be great.

[djc]

(With the suggestion that I recently read an opinion that "responsible disclosure" is maybe not the best name, and "coordinated disclosure" is maybe more accurate.)