-
Notifications
You must be signed in to change notification settings - Fork 3
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
Remove the locked version of sysrandom #1
base: master
Are you sure you want to change the base?
Conversation
Currently this gem uses https://github.com/cryptosphere/sysrandom, which is a version of securerandom that has been dead since 2018. It seems to have been used as a workaround to this long running issue (from 2014) which looks to now be long resolved - https://bugs.ruby-lang.org/issues/9569 This is an issue for 2 reasons: - Relying on unsupported gems might result in security issues (unlikely in something like this, but worth keeping in mind) - It appears to overload SecureRandom, which breaks a bunch of other gems that rely on randomisation If applied this commit will remove this locked version and start relying on the ruby stdlib version of securerandom instead which is supported
+1 the old gem usage leads to unexpected problem with random numbers in multiple libs/gems. Because |
👍 The dependency on |
This change is from: fastlane#1
I forked this gem and made the changes in this PR in the fork. The changes have now been in my production app for ~24 hours and it has resolved the problems I was having. |
@joshdholtz any chance you could take a look at this fix? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need this merged as well. For some reason, this triggers an issue in some github fastlane actions.
Currently this gem uses https://github.com/cryptosphere/sysrandom, which is a version of securerandom that has been dead since 2018. It seems to have been used as a workaround to this long running issue (from 2014) which looks to now be long resolved - https://bugs.ruby-lang.org/issues/9569
This is an issue for 2 reasons:
If applied this commit will remove this locked version and start relying on the ruby stdlib version of securerandom instead which is supported and relied on by other gems
DISCLAIMER: Unfortunately I'm not a security expert, so I could well be missing a detail on this. But as far as I can tell the issue referenced is long resolved.
I've managed to run this fork to be able to log in to ASC and collect review and app data successfully