#4254 CVE-2019-7628: Do not leak partial API keys.
Merged 7 days ago by pingou. Opened 11 days ago by bowlofeggs.
bowlofeggs/pagure dont_email_secrets  into  master

file modified
+2 -3

@@ -45,7 +45,6 @@ 

              user = token.user

              username = user.fullname or user.username

              user_email = user.default_email

-             api_key = token.id

              days_left = (token.expiration - datetime.utcnow()).days

              subject = 'Pagure API key expiration date is near!'

              if token.project:

@@ -57,7 +56,7 @@ 


  Your Pagure Admin. ''' % (


-                     api_key[:5],

+                     token.description,




@@ -69,7 +68,7 @@ 


  Your Pagure Admin. ''' % (


-                     api_key[:5],

+                     token.description,


              if not check:

                  msg = pagure.lib.notify.send_email(text, subject, user_email)

It was discovered that Pagure was leaking API keys by e-mailing
them to users. Few e-mail servers validate TLS certificates, so
it is possible for man-in-the-middle attacks to read these e-mails
and gain access to Pagure on the behalf of other users. The
vulnerability was introduced in [0].

This problem was partially addressed in a prior commit[1], but
that commit still leaks the first 5 characters of the key which
weakens the secret.

This commit uses the description of the API key instead of any part
of the secret in the e-mail sent to users so that none of the key
is e-mailed over the Internet.

[0] 57975ef
[1] 9905fb1

fixes #4253

Signed-off-by: Randy Barlow randy@electronsweatshop.com

rebased onto 904c0fc

11 days ago


This issue can be worked around by disabling the cron job. After disabling the cron job, it would be wise to delete any API keys you think may have been e-mailed. You can delete them all to be safe. Users will have to generate new ones if you take this step.

rebased onto c7a41f0

11 days ago

rebased onto d4dff60

8 days ago

pretty please pagure-ci rebuild

7 days ago

rebased onto d18f42d

7 days ago

The tests failed but because of repoSpanner which is broken for everyone.

I'll adjust the wording of the email sent a little bit as well but that can come in another PR

Thanks for the patch and for catching this and doing all the work to get the CVE number and all!

Pull-Request has been merged by pingou

7 days ago