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.

Sign up for GitHub

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

Jump to bottom

Update mopa to avoid safety issue (no longer maintained) #217

Closed
Cypher1 opened this issue Jul 10, 2022 · 6 comments · Fixed by #218
Closed

Update mopa to avoid safety issue (no longer maintained) #217

Cypher1 opened this issue Jul 10, 2022 · 6 comments · Fixed by #218
Labels
bug

Comments

@Cypher1
Copy link

Cypher1 commented Jul 10, 2022 •
edited
Loading

Dependabots recently opened an advisory on one of my projects https://github.com/Cypher1/tako/ because I'm using specs, which depends on shred, which depends on a version of mopa, whose latest version contains an unsound pattern which can cause segfaults.

Issue here:
chris-morgan/mopa#13

Currently the thread's contributors state that mopa is not maintained, but one has set up mopa-maintained, as an alternative where they have added a fix.

Please investigate and update your deps so we can all avoid potential for segfaults :)

Cheers!
@Cypher1
Copy link
Author

Cypher1 commented Jul 10, 2022

Links to dependabots security issues:

https://github.com/Cypher1/tako/security/dependabot/1
https://github.com/Cypher1/tako/security/dependabot/4

@torkleyy
Copy link
Member

Thanks for reporting the issue @Cypher1! At this point, I'm wondering if the dependency is even worth it, we're using the macro once in the entire crate, maybe that code should just be inlined instead? Would you be willing to create a PR for that?

@torkleyy torkleyy added the bug label Jul 10, 2022
@torkleyy
Copy link
Member

Interesting that you're building a compiler with an ECS, I was thinking about the same thing yesterday, since I'm writing a compiler for a toy language myself. It seemed cumbersome to repeat the structures of your language for every intermediate representation, so I thought why not assign everything an ID and use tables to store related information for the current phase. And ECS seems like the perfect fit for that :)

@torkleyy
Copy link
Member

FYI, there's also bevy_ecs, which I think can be used independently and is more actively maintained.

torkleyy added a commit that referenced this issue Jul 10, 2022
@torkleyy
Eliminate safety issue in Resource::downcast*
2984837 
- The previous implementation uses `mopa`, which has a known safety
issue: chris-morgan/mopa#13
- This commit essentially inlines the code generated by
the `mopafy!` macro provided by `mopa`, thus removing the
dependency
- Since `mopa::Any` was part of the public interface of the `Resource`
trait, this is technically a breaking change

Fixes #217
@torkleyy torkleyy mentioned this issue Jul 10, 2022
Merged
@torkleyy
Copy link
Member

I went ahead an created a PR: #218

@Cypher1
Copy link
Author

Cypher1 commented Jul 10, 2022

Thanks @torkleyy I'll check out bevy ECS. I'm really glad you opened the pr as I don't know my way around shred internals

@AcrylicShrimp AcrylicShrimp mentioned this issue Dec 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Eliminate safety issue in Resource::downcast* amethyst/shred
2 participants
@Cypher1 @torkleyy