On 05/20/2015 10:38 AM, Andrew Shadura wrote:
Hello,

On Tue, 19 May 2015 17:10:46 +0200
Mads Kiilerich <m...@kiilerich.com> wrote:
@@ -296,10 +300,19 @@ class UserModel(BaseModel):
user_email = data['email']
           user = User.get_by_email(user_email)
+        timestamp = int(time.time())
           if user:
               log.debug('password reset user found %s' % user)
+            token = hashlib.sha1('%s%s%s%s%s' % (user.user_id,
+                                               timestamp,
+                                               user.password,
+                                               session.id,
+
config.get('app_instance_uuid'))
+            ).hexdigest()
Now we are reinventing crypto. That is dangerous. One thing: One
thing is that hashes could give conflicts in the hash but we should
make sure that we never got "collisions" in the input string - such
collisions could perhaps be used as attacks. Perhaps use \n as
delimiter of the fields?
Well, none of the data we use here is accepted directly from user
(except timestamp — when verifying the token; I should probably
validate it before using). From what I saw in similar cases elsewhere
(for example, OAuth 1.0), sometimes the data is escaped to avoid
collisions.

I agree ... but adding a separator that can't occur in the inputs would make the code more resilient to future errors


By the way, for the record to others: We will need a similar scheme for verifying email addresses configured by users.

/Mads
_______________________________________________
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Reply via email to