BluezoneGlobal / react-native-bluetooth-scan

Bluezone - Bảo vệ mình, bảo vệ cộng đồng
https://bluezone.ai
GNU General Public License v3.0
54 stars 41 forks source link

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

Closed htruong closed 4 years ago

htruong commented 4 years ago

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.

khanguy00 commented 4 years ago

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()! })
}
BkavMS17 commented 4 years ago

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 commented 4 years ago

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