django-rest-framework
                                
                                 django-rest-framework copied to clipboard
                                
                                    django-rest-framework copied to clipboard
                            
                            
                            
                        There's an infinitesimally small possibility an existing token gets overwritten.
https://github.com/encode/django-rest-framework/blob/5ad467aa2c76a7cfb6218f6dbe22e314131bde3c/rest_framework/authtoken/models.py#L9-L37
The sample space is large 2 ** (8 * 20), but the probability is not 0 that generate_key returns a key that already exists, and then the save would update the existing one.
The smallest change, turns this into an integrity error:
     def save(self, *args, **kwargs): 
         if not self.key: 
             self.key = self.generate_key()
             kwargs["force_insert"] = True
         return super().save(*args, **kwargs) 
Yep that looks like a very sensible measure. Would you like to go ahead and raise a pull request for this?
Reassuringly "large" is an understatment here. But yes, the force_insert/IntegrityError makes sense.
We should really be pointing towards our security policy and following issues like this up promptly, but the project maintainership has been severely over-extended recently. We're now getting back onto a more even keel.
I'm not really into this project, but I saw this issue...
To the author of this issue: you shouldn't report security issues publicly.
To the maintainers: this should have been closed as soon as this was read, and recreated on another communication channel (email, private forum, etc) for triage.
This isn't an exploitable security issue in reality, tho we ought to have been be keeping good policy regardless yes.
Should this fix be considered a potential breaking change? e.g if some folks relied on the current behaviour to rotate an existing token. They would clear the key field and call save() on an existing token.
Should this fix be considered a potential breaking change? e.g if some folks relied on the current behaviour to rotate an existing token. They would clear the
keyfield and callsave()on an existing token.
Would that even work? I'll test that when I prepare the PR.