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

The random ID generator for iOS is implemented incorrectly, generates a biased random number #5

Closed
htruong opened this issue Apr 28, 2020 · 3 comments

Comments

@htruong
Copy link

htruong commented Apr 28, 2020

The Swift code to generate the random ID for iOS is wrong.

The arc4random_uniform function takes one parameter, which is the upper bound of the number you want to generate (Here - I couldn't find the official docs on Apple's website, but it seems like it's depreciated by random -- which is called the same way). Here, you want to select a random position on your charRandom, you have to call let valueRandom = Int32(arc4random_uniform( charRandom.count ))), instead of calling it with timeIntervalSince1970.

The developer here seemed to have thought that the function needs the seed specified. This function does not need the seed to be specified. Apple docs noted: "This method is equivalent to calling the version that takes a generator, passing in the system’s default random generator."

Now, the problem with giving it the wrong upper bound is that your random number will be biased when you do the modulo. For example, when you have a string of 10 characters 0123456789 and you give the generator the upper bound of 11 then do the modulo 10 function, then you're ever so slightly more likely to select 0 than every other character. See modulo bias. It should not matter much practically when your upper bound is large, but it's alarming to see someone misunderstanding such a fundamental concept.

PS: Even if the function took the parameter as the seed, this function would still have been wrong. According to Apple docs, this function yields the number of seconds since 1970. This means given the function runs under a second, then the valueRandom will stay the same and you would end up with an ID that looks like AAAAAAAAAA.

@htruong htruong changed the title The random ID generator for iOS code is wrong The random ID generator for iOS is wrong Apr 28, 2020
@khanguy00
Copy link

khanguy00 commented Apr 28, 2020

So, I think what @htruong suggests is:

for _ in 0 ..< length {
    let index = arc4random_uniform(charRandom.count)
    bluezonerId += String(charRandom[charRandom.index(charRandom.startIndex, offsetBy: index)])
}

That's good then. But there is a shorter way (Swift 4.2+):

func createBluezonerId(numberChar: Int) -> String {
  let letters = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
  return String((0..<numberChar).map{ _ in letters.randomElement()! })
}

@htruong htruong changed the title The random ID generator for iOS is wrong The random ID generator for iOS is implemented wrongly, generates a biased random number Apr 28, 2020
@htruong htruong changed the title The random ID generator for iOS is implemented wrongly, generates a biased random number The random ID generator for iOS is implemented incorrectly, generates a biased random number Apr 28, 2020
@BkavMS17
Copy link

BkavMS17 commented Aug 4, 2020

Sorry for missing this topic. In the current Bluezone version implemented in Vietnam, we no longer use arc4random function to create BluezoneId. Now, BluezoneId is generated through the SHA256 hash function with a random key generated initially according to the operating system's random key generation function.
Thanks,

@htruong
Copy link
Author

htruong commented Aug 4, 2020

Thanks, it would be great to have the source code published too. I'm closing this ticket since it's no longer an issue.

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

3 participants