CVE-2020-14423 (Other, published 2020-06-18T14:15Z)
Description: value: Convos before 4.20 does not properly generate a random secret in Core/Settings.pm and Util.pm. This leads to a predictable CONVOS_LOCAL_SECRET value, affecting password resets and invitations.
Severity: MEDIUM (CVSS v3 base score: 5.3)
Weakness: CWE-330 \u2014 Use of Insufficiently Random Values
The product uses insufficiently random numbers or values in a security context that depends on unpredictable numbers.
Repository: https://github.com/Nordaaker/convos
Files changed:
lib/Convos/Core/Settings.pm
lib/Convos/Util.pm
t/config.t
t/util.t
Commit message: Fix generating random local secret for Docker


Vulnerable code:
use Mojo::Util;
has local_secret   => \&_build_local_secret;


  # Remove legacy secrets file
  my $file = $self->_legacy_session_secrets_file;
  $file->remove if -e $file;

sub _build_local_secret {
  my $self = shift;
  return $ENV{CONVOS_LOCAL_SECRET} if $ENV{CONVOS_LOCAL_SECRET};

  my $secret = Mojo::Util::md5_sum(join ':', $self->core->home->to_string, $<, $(, $0);
  return $secret;
}

  my $secrets;
  my $file = $self->_legacy_session_secrets_file;
  $secrets = [split /‚/, $file->slurp] if -e $file;
  return $secrets if $secrets and @$secrets;
  $secrets = [Mojo::Util::sha1_sum(join ':', rand(), $$, $<, $(, $^X, Time::HiRes::time())];
  return $secrets;
sub _legacy_session_secrets_file { shift->core->home->child('secrets') }

use Mojo::File;
use Mojo::Util qw(b64_decode b64_encode md5_sum monkey_patch);
use Time::Piece ();
  qw($CHANNEL_RE DEBUG E has_many pretty_connection_name require_module),
  qw(sdp_decode sdp_encode short_checksum tp),
like $convos->settings('local_secret'), qr/^\w{32}$/, 'generated local_secret';
use Convos::Util qw(require_module sdp_decode sdp_encode short_checksum);

Fixed code:
use Convos::Util 'generate_secret';
has local_secret   => sub { $ENV{CONVOS_LOCAL_SECRET} || generate_secret };
  my $file    = $self->core->home->child('secrets');
  my $secrets = -r $file ? [split /‚/, $file->slurp] : [];
  $file->remove if -e $file;
  return @$secrets ? $secrets : [generate_secret];
use Mojo::Util qw(b64_decode b64_encode md5_sum monkey_patch sha1_sum);
use Sys::Hostname ();
use Time::HiRes   ();
use Time::Piece   ();
  qw($CHANNEL_RE DEBUG E generate_secret has_many pretty_connection_name),
  qw(require_module sdp_decode sdp_encode short_checksum tp),
sub generate_secret {
  return eval { _generate_secret_urandom() } || _generate_secret_fallback();
}

sub _generate_secret_fallback {
  return sha1_sum join ':', rand(), $$, $<, Sys::Hostname::hostname(), Time::HiRes::time();
}

sub _generate_secret_urandom {
  my $len = shift || $ENV{CONVOS_SECRET_URANDOM_READ_LEN} || 128;
  open my $fh, '<', '/dev/urandom' or die "Can't open /dev/urandom: $!";
  my $ret = sysread $fh, my ($secret), $len;
  return sha1_sum $secret if $ret == $len;
  die qq{Could not read $len bytes from "/dev/urandom": $!};
}

=head2 generate_secret

  $str = generate_secret;

Returns a SHA1 sum of bytes from "/dev/urandom" or fallback to a SHA1 sum of:

    rand()     # Not cryptographically secure, but pseudo random
    $$         # Will probably be "1" inside Docker and probably less than 32768 on Linux
    $<         # Will probably be "0" inside Docker and probably less than 10000 on Linux
    hostname() # Will be unique inside Docker and guessable on Linux
    time()     # Floating seconds since the epoch (Ex: 1592094819.82439)

like $convos->settings('local_secret'), qr/^\w{40}$/, 'generated local_secret';
use Convos::Util qw(generate_secret require_module sdp_decode sdp_encode short_checksum);
note 'generate_secret';
is length(generate_secret), 40, 'generate_secret';
my %secrets;
map { $secrets{+Convos::Util::_generate_secret_urandom()}++ } 1 .. 1000 if -r '/dev/urandom';
map { $secrets{+Convos::Util::_generate_secret_fallback()}++ } 1 .. 1000;
is_deeply [values %secrets], [map {1} values %secrets],
  '1..1000 is not nearly enough to prove anything, but testing it anyways';