Bug#886593: stretch-pu: package qtpass/1.1.6-1

[Adam D. Barratt]

Control: tags -1 + pending

On Fri, 2018-02-23 at 17:59 +, Adam D. Barratt wrote:
> Control: tags -1 + confirmed
> 
> On Mon, 2018-01-08 at 00:17 +0100, Philip Rinn wrote:
> > the current version in stable has a insecure built-in password
> > generator. As
> > the built-in password generator not used in qtpass' default config,
> > the
> > security team asked me to fix it via stretch-pu.
> 
> Please go ahead.
> 

Uploaded and flagged for acceptance.

Regards,

Adam

Bug#886593: stretch-pu: package qtpass/1.1.6-1

[Adam D. Barratt]

Control: tags -1 + confirmed

On Mon, 2018-01-08 at 00:17 +0100, Philip Rinn wrote:
> the current version in stable has a insecure built-in password
> generator. As
> the built-in password generator not used in qtpass' default config,
> the
> security team asked me to fix it via stretch-pu.

Please go ahead.

Regards,

Adam

Bug#886593: stretch-pu: package qtpass/1.1.6-1

[Philip Rinn]

Package: release.debian.org
Severity: normal
Tags: stretch
User: release.debian@packages.debian.org
Usertags: pu

Hi,

the current version in stable has a insecure built-in password generator. As
the built-in password generator not used in qtpass' default config, the
security team asked me to fix it via stretch-pu.
Here is the corresponding link:
https://security-tracker.debian.org/tracker/source-package/qtpass

I attached the debdiff (the fix is adopted from upstream, see
https://github.com/IJHack/QtPass/issues/338 for reference).

May a go ahead?

Best,

Philip
diff -Nru qtpass-1.1.6/debian/changelog qtpass-1.1.6/debian/changelog
--- qtpass-1.1.6/debian/changelog   2016-12-02 16:23:16.0 +0100
+++ qtpass-1.1.6/debian/changelog   2018-01-07 13:45:10.0 +0100
@@ -1,3 +1,9 @@
+qtpass (1.1.6-1+deb9u1) stretch; urgency=medium
+
+  * Fix insecure built-in password generator (Fixes: CVE-2017-18021)
+
+ -- Philip Rinn   Sun, 07 Jan 2018 13:45:10 +0100
+
 qtpass (1.1.6-1) unstable; urgency=medium
 
   * New upstream release
diff -Nru qtpass-1.1.6/debian/NEWS qtpass-1.1.6/debian/NEWS
--- qtpass-1.1.6/debian/NEWS1970-01-01 01:00:00.0 +0100
+++ qtpass-1.1.6/debian/NEWS2018-01-07 13:45:10.0 +0100
@@ -0,0 +1,15 @@
+qtpass (1.1.6-1+deb9u1) stretch; urgency=medium
+
+  All passwords generated with QtPass' built-in password generator prior to
+  1.1.6-1+deb9u1 are possibly predictable and enumerable by hackers.
+  The generator used libc's random(), seeded with srand(msecs), where msecs is
+  not the msecs since 1970 (not that that'd be secure anyway), but rather the
+  msecs since the last second. This means there are only 1000 different
+  sequences of generated passwords.
+  .
+  NB: QtPass uses `pwgen` to generate passwords by default. This means, if you
+  didn't change the configuration to use the built-in password generator your
+  passwords are safe. If you used the built-in password generator, change all
+  passwords you generated with QtPass.
+
+ -- Philip Rinn   Sun, 07 Jan 2018 13:45:10 +0100
diff -Nru qtpass-1.1.6/debian/patches/01-fix-password-generator.patch 
qtpass-1.1.6/debian/patches/01-fix-password-generator.patch
--- qtpass-1.1.6/debian/patches/01-fix-password-generator.patch 1970-01-01 
01:00:00.0 +0100
+++ qtpass-1.1.6/debian/patches/01-fix-password-generator.patch 2018-01-04 
22:38:41.0 +0100
@@ -0,0 +1,67 @@
+--- a/mainwindow.cpp
 b/mainwindow.cpp
+@@ -67,7 +67,6 @@
+   connect(actionAddPassword, SIGNAL(triggered()), this,
+   SLOT(on_addButton_clicked()));
+   connect(actionAddFolder, SIGNAL(triggered()), this, SLOT(addFolder()));
+-  qsrand(static_cast(QTime::currentTime().msec()));
+ 
+ #if QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)
+   ui->lineEdit->setClearButtonEnabled(true);
+@@ -1900,10 +1899,10 @@
+ else
+   qDebug() << "pwgen fail";
+   } else {
+-int charsetLength = pwdConfig.Characters[selection].length();
++quint32 charsetLength = pwdConfig.Characters[selection].length();
+ if (charsetLength > 0) {
+   for (int i = 0; i < length; ++i) {
+-int index = qrand() % charsetLength;
++quint32 index = Util::boundedRandom(charsetLength);
+ QChar nextChar = pwdConfig.Characters[selection].at(index);
+ passwd.append(nextChar);
+   }
+--- a/util.cpp
 b/util.cpp
+@@ -9,6 +9,9 @@
+ #else
+ #include 
+ #endif
++#include 
++#include 
++#include 
+ QProcessEnvironment Util::_env;
+ bool Util::_envInitialised;
+ 
+@@ -137,3 +140,21 @@
+   nanosleep(, NULL);
+ #endif
+ }
++
++quint32 Util::boundedRandom(quint32 bound) {
++  static int fd = -1;
++  if (bound < 2)
++  return 0;
++
++  if (fd == -1)
++  assert((fd = open("/dev/urandom", O_RDONLY)) >= 0);
++
++  quint32 randval;
++  const quint32 max_mod_bound = (1 + ~bound) % bound;
++
++  do
++  assert(read(fd, , sizeof(randval)) == sizeof(randval));
++  while (randval < max_mod_bound);
++
++  return randval % bound;
++}
+--- a/util.h
 b/util.h
+@@ -16,6 +16,7 @@
+   static bool checkConfig(QString passStore, QString passExecutable,
+   QString gpgExecutable);
+   static void qSleep(int ms);
++  static quint32 boundedRandom(quint32 bound);
+ 
+ private:
+   static void initialiseEnvironment();
diff -Nru qtpass-1.1.6/debian/patches/series qtpass-1.1.6/debian/patches/series
--- qtpass-1.1.6/debian/patches/series  1970-01-01 01:00:00.0 +0100
+++ qtpass-1.1.6/debian/patches/series  2018-01-04 22:11:50.0 +0100
@@ -0,0 +1 @@
+01-fix-password-generator.patch