Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mitigate risk of losing vulnerability fix #102? #103

Open
8573 opened this issue Mar 19, 2023 · 2 comments
Open

Mitigate risk of losing vulnerability fix #102? #103

8573 opened this issue Mar 19, 2023 · 2 comments

Comments

@8573
Copy link
Contributor

8573 commented Mar 19, 2023

#102 fixes a vulnerability that has been named RUSTSEC-2023-0021 (rustsec/advisory-db#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
Copy link
Contributor

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:

rust-stb-image/src/stb_image.c

Lines 3828 to 3835 in ddb1223

if (!pic_load2(s,x,y,comp, result)) {
free(result);
result=0;
}
*px = x;
*py = y;
if (req_comp == 0) req_comp = *comp;
result=convert_format(result,4,req_comp,x,y);

@8573
Copy link
Contributor Author

8573 commented Mar 20, 2023

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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants