servo / rust-stb-image

Rust bindings to the awesome stb_image library
Other
68 stars 34 forks source link

Mitigate risk of losing vulnerability fix #102? #103

Open 8573 opened 1 year ago

8573 commented 1 year ago

https://github.com/servo/rust-stb-image/pull/102 fixes a vulnerability that has been named RUSTSEC-2023-0021 (https://github.com/rustsec/advisory-db/pull/1647). The fix is made by directly changing a vendored C file.

This fix seems liable to get lost the next time the vendored C file is updated to a new upstream version, as it appears to have been the last time it was changed.

Would it be reasonable/cost-effective for Servo to take a step to mitigate this risk, such as

mbrubeck commented 1 year ago

Would it be reasonable/cost-effective for Servo to take a step to mitigate this risk, such as

  • patching the C file at build-time instead

Yes, that would be great. If you are interested in submitting a PR for this, I'd be happy to review it. (If not, I or another maintainer can do it when we have time.)

This fix seems liable to get lost the next time the vendored C file is updated to a new upstream version, as it appears to have been the last time it was changed.

Just to be clear, nothing was lost in that upgrade, since no patches had been applied to the upstream code prior to that commit. The vulnerable code existed unfixed in the parent revision as well:

https://github.com/servo/rust-stb-image/blob/ddb122381df6249f7916c3cdadd9f42396d36602/src/stb_image.c#L3828-L3835

8573 commented 1 year ago

This fix seems liable to get lost the next time the vendored C file is updated to a new upstream version, as it appears to have been the last time it was changed.

Just to be clear, nothing was lost in that upgrade

Yes, apologies, I meant "updated ... as it appears to have been", not "lost ... as it appears to have been".