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

Remove the locked version of sysrandom #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CDeighton
Copy link

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 (this is my selfish case 🙂)

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

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
@Loriowar
Copy link

+1 the old gem usage leads to unexpected problem with random numbers in multiple libs/gems. Because random_number method in the old gem allows to pass only number, not range. In the modern implementation of random_number from Random::Formatter there are three variants of the input args (no arg, int or range). However, the outdated sysrandom has only two (no arg or int).

@kstevens715
Copy link

kstevens715 commented Oct 25, 2024

👍 The dependency on sysrandom is causing a lot of problems in my app in areas completely unrelated to fastlane. A similar issue was opened in the main fastlane repo here: fastlane/fastlane#26682, and another sysrandom related issue here: fastlane/fastlane#26655.

kstevens715 added a commit to modolabs/fastlane-sirp that referenced this pull request Oct 28, 2024
This change is from: fastlane#1
This was referenced Oct 28, 2024
@kstevens715
Copy link

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.

@christophby
Copy link

@joshdholtz any chance you could take a look at this fix?

Copy link

@TizianoCoroneo TizianoCoroneo left a 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.

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

Successfully merging this pull request may close these issues.

5 participants