id summary reporter owner description type status component version severity resolution keywords cc stage has_patch needs_docs needs_tests needs_better_patch easy ui_ux 35730 Enhance password reset security by encrypting 'uid' parameter instead of base64-encoding to prevent possible user count leakage Remy Remy "When using Django’s default view for requesting a password reset, `PasswordResetView`, the `PasswordResetForm`’s save() method sends an email containing a `uid` parameter generated using `urlsafe_base64_encode(force_bytes(user.pk))`. This results in the user’s email inbox containing a password reset link that indirectly reveals the user’s primary key (`user.pk`), which exposes information about how many users exist on any Django site that uses this default view. Surely, organizations that design their entities with non-enumerable public identifiers (such as by using a `UUIDField` for the primary key) would not be affected by this, however as the issue is also addressed by other means, such as a secondary public identifier, or simply a careful app design, I would still think that many Django site owners who prefer to keep this information private are likely unaware that it’s being exposed through this native mechanism. To prevent the leakage of the `user.pk` value by default, I replaced the base64 encoding with the ~~signing~~ encrypting of the `user.pk` value (PR https://github.com/django/django/pull/18539). **UPDATE**: moved on from signing to encrypting. Details bellow: **Implementation details** Since Python’s standard library does not include encryption utilities, and because the goal is only to obfuscate data that was previously not obfuscated, the need for a highly secure encryption algorithm is somewhat mitigated. Adding a new dependency to Django just for this seemed unnecessary. Therefore, I opted for a straightforward implementation of a XOR cipher. The cipher key is derived from Django’s `SECRET_KEY` and salted. **Security considerations** In the case of a XOR cipher, the worst-case scenario is that an attacker manages to obtain the cipher key. However, even if this happened, all the attacker would be able to do is decrypt the `user.pk`, which is currently being sent almost as plain text in the default implementation anyway. Regarding other potential uses of the key: - Since the key is derived from a salted hash of the `SECRET_KEY`, it is not reversible, and the salt protects against rainbow table attacks (for example, if `SECRET_KEY` was something weak like ""abc""). - The key is not reused anywhere else in the application and remains scoped only to this encryption/decryption functionality, ensuring the attack surface is limited to this single feature. It’s also worth noting that it is highly unlikely that an attacker would even obtain the full cipher key to begin with, due to the nature of XOR encryption. To do so, they would need to use plain-text messages that are at least as long as the cipher key. For this purpose, we use a `SHA-512` derived key, which is 64 bytes long, while most `user.pk` values use `BigAutoField` which maximum value `9223372036854775807` is only 19 bytes. An implementation of `user.pk` using `UUIDField` and a UUID4 would also be significantly shorter (36 bytes). We could go further and introduce a salt that changes with each request. For instance, we could use a timestamp as the salt during encryption and decryption. The timestamp would travel with the reset link and return with the request. We actually already have such a timestamp traveling within the `token` parameter (`ts_b36`). So if we were to go this route, it would be logical to re-use this `ts_b36` for salting the cipher key, and we could even include the encypted uid in the token parameter itself (along with `f'{ts_b36}-{hash_string}'`) and remove `uidb64` from url parameters. However, this dynamic salting would require rewriting the logic inside `PasswordResetConfirmView` and I would argue that we're already doing too much since we're really just obfuscating something that was in clear until now. With the current implementation, we achieve most of the desired benefits without needing to rewrite that `PasswordResetConfirmView` logic, and the URL parameters remain unchanged (`reset///`)." Cleanup/optimization assigned contrib.auth dev Normal Remy Accepted 1 1 0 1 0 0