Skip to content

fix(web-pkg)! creating biased random numbers from a cryptographically secure source #12575

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

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

Conversation

odaysec
Copy link

@odaysec odaysec commented May 18, 2025

const j = Math.floor((window.crypto.getRandomValues(new Uint8Array(1))[0] / 256) * (i + 1))

Fix the issue need to replace the biased division-based random index generation with a method that ensures uniform distribution. This can be achieved by using a rejection sampling approach, similar to the one used earlier in the getRandomCharsFromSet function. Specifically, we will calculate a setLimit for the array length and discard random values that fall outside this range. This ensures that the modulo operation produces unbiased results.

The changes will be made to the shuffle logic on lines 184–186.


References

Understanding “randomness”
Insecure Randomness
Rule - Use strong approved cryptographic algorithms
CWE-327

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

Open tasks:

  • ...

Copy link

update-docs bot commented May 18, 2025

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@CLAassistant
Copy link

CLAassistant commented May 18, 2025

CLA assistant check
All committers have signed the CLA.

@LukasHirt LukasHirt self-requested a review May 19, 2025 09:07
@LukasHirt LukasHirt added the Type:Bug Something isn't working label May 19, 2025
@LukasHirt
Copy link
Collaborator

Hello @odaysec, thank you for the contribution! Two small things. The CI lint pipeline is broken due to formatting. Also, we would kindly ask you to provide a changelog item https://github.com/owncloud/web/blob/master/changelog/README.md#create-changelog-items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants