Problèmes d’implémentation (faux mélange, Off by One error) dans la génération des codes alphanumériques des tests COVID
The following bug report has been received through the YesWeHack private bugbounty phase.
Acknowledgements: Baptiste Robert
Dû a des problèmes d’implémentation le code alphanumérique correspondant à un test COVID n’est pas tiré uniformément. En effet, dans la classe AlphaNumericCodeServiceImpl.java contenu dans l’archive soumission-code-server_snapshot.tar.gz, disponible sur le Gitlab de l’INRIA https://gitlab.inria.fr/stopcovid19/submission-code-server, on peut trouver 2 erreurs qui biaisent la distribution
Faux mélange
protected static List<Character> getShuffledAlphaNumList() {
final ArrayList<Character> tempAlphaNumList = new ArrayList<>(ALPHA_NUMERIC_CHAR_ARRAY);
Collections.shuffle(ALPHA_NUMERIC_CHAR_ARRAY,sRandom);
return tempAlphaNumList;
}
Code in src\main\java\fr\gouv\stopc\submission\code\server\commun\service\impl\AlphaNumericCodeServiceImpl.java:46
La méthode getShuffledAlphaNumList ne remplit pas sa fonction car au lieu de mélanger la liste nouvellement créé, tempAlphaNumList, elle mélange ALPHA_NUMERIC_CHAR_ARRAY. Par conséquent, elle renverra toujours une liste ordonnée abcdefghijklmnopqrstuvwxyz0123456789.
Pour résoudre ce problème, il faut modifier la fonction comme ceci:
protected static List<Character> getShuffledAlphaNumList() {
final ArrayList<Character> tempAlphaNumList = new ArrayList<>(ALPHA_NUMERIC_CHAR_ARRAY);
Collections.shuffle(tempAlphaNumList,sRandom);
return tempAlphaNumList;
}
Off by One Error
public String generateCode() {
log.info("Generating random 6-alphanum code");
final List<Character> characters = getShuffledAlphaNumList();
String alphaNum = "";
for (int i = 0; i < CODE_SIZE; i++) {
alphaNum += characters.get(sRandom.nextInt(ALPHA_NUMERIC_CHAR_ARRAY.size()-1)).toString();
}
return alphaNum;
}
Code in src\main\java\fr\gouv\stopc\submission\code\server\commun\service\impl\AlphaNumericCodeServiceImpl.java:32
Il y a une erreur d’off by one dans la fonction generateCode. En effet, la ligne suivante, characters.get(sRandom.nextInt(ALPHA_NUMERIC_CHAR_ARRAY.size()-1)), ne sélectionnera jamais le dernier caractère.
POC Vous trouverez ci-dessous un POC qui met en lumière ces 2 problèmes.
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
public class poc {
private static final String ALPHA_UPPER_CASE = "abcdefghijklmnopqrstuvwxyz".toUpperCase();
private static final String NUMERIC= "0123456789";
private static final Integer CODE_SIZE = 6;
private static final SecureRandom sRandom = new SecureRandom();
public static void main(String[] args) {
System.out.println(generateCode());
}
private static final List<Character> ALPHA_NUMERIC_CHAR_ARRAY = String
.format(
"%s%s",
ALPHA_UPPER_CASE,
NUMERIC)
.chars()
.mapToObj(c -> (char) c)
.collect(Collectors.toList());
public static String generateCode() {
System.out.println("Generating random 6-alphanum code");
final List<Character> characters = getShuffledAlphaNumList();
for (int i = 0; i < characters.size(); i++) {
System.out.println(Integer.toString(i) + ": " + characters.get(i));
}
System.out.println("characters size = " + characters.size());
System.out.println("ALPHA_NUMERIC_CHAR_ARRAY size - 1 = " + Integer.toString((ALPHA_NUMERIC_CHAR_ARRAY.size()-1)));
System.out.println(characters.get((ALPHA_NUMERIC_CHAR_ARRAY.size() - 1)) + " is unreachable");
String alphaNum = "";
for (int i = 0; i < CODE_SIZE; i++) {
alphaNum += characters.get(sRandom.nextInt(ALPHA_NUMERIC_CHAR_ARRAY.size()-1)).toString();
}
return alphaNum;
}
/**
* @return return a shuffled copy of ALPHA_NUMERIC_CHAR_ARRAY
*/
protected static List<Character> getShuffledAlphaNumList() {
final ArrayList<Character> tempAlphaNumList = new ArrayList<>(ALPHA_NUMERIC_CHAR_ARRAY);
Collections.shuffle(ALPHA_NUMERIC_CHAR_ARRAY,sRandom);
return tempAlphaNumList;
}
}
Conclusion La conjonction de ces 2 problèmes est que le code correspondant à un test Covid n’est pas tiré uniformément puisqu’il ne comportera jamais de 9