Session ids should really be hard to guess so a user cannot take over another's session. This also fixes a bug where the session id length could be less than 32 characters.
Note: * This introduces a dependency on the Bytes::Random::Secure Perl module.
Signed-off-by: Francois Gouget fgouget@codeweavers.com ---
libbytes-random-secure-perl is already installed so this should just take a web server restart.
Perl documents rand() as being not cryptographically secure. That means an attacker could cycle through session ids for his own account and derive the random number generator internal state, thus allowing him to predict the session ids for other users' next login.
Funnily enough the bug that caused session ids to not always have 32 characters could actually make this type of attack much harder: for this attack one needs the sequence of values returned by rand(), here groups of 4 hexadecimal characters. But because sprintf("%lx") does not 0-pad its result, a 31 charachter session id starting with '123456789ab...' could either be '123' + '4567' + '89ab', or '1234' + '567' + '89ab', or '1234' + '5678' + '9ab', etc.
But not being a cryptographer and not knowing the internals of rand() I don't know how much harder this makes the attacks. So better safe than sorry.
testbot/doc/INSTALL.txt | 1 + testbot/lib/WineTestBot/CGI/Sessions.pm | 8 +++----- 2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/testbot/doc/INSTALL.txt b/testbot/doc/INSTALL.txt index 456ffe6957..4d430d9157 100644 --- a/testbot/doc/INSTALL.txt +++ b/testbot/doc/INSTALL.txt @@ -6,6 +6,7 @@ Dependencies: - MySQL - Perl DBD and DBI::mysql modules - Sendmail and Procmail +- Bytes::Random::Secure (libbytes-random-secure-perl) - Sys::Virt (libsys-virt-perl, see http://libvirt.org/) - Image::Magick (perlmagick) - Optional: IO::Socket::IP (for IPv6, libio-socket-ip-perl) diff --git a/testbot/lib/WineTestBot/CGI/Sessions.pm b/testbot/lib/WineTestBot/CGI/Sessions.pm index 8cd3c52fb7..4ca62ba76e 100644 --- a/testbot/lib/WineTestBot/CGI/Sessions.pm +++ b/testbot/lib/WineTestBot/CGI/Sessions.pm @@ -54,7 +54,9 @@ use WineTestBot::WineTestBotObjects; our @ISA = qw(WineTestBot::WineTestBotCollection); our @EXPORT = qw(CreateSessions DeleteSessions NewSession);
+use Bytes::Random::Secure; use CGI::Cookie; + use ObjectModel::BasicPropertyDescriptor; use ObjectModel::ItemrefPropertyDescriptor; use WineTestBot::Users; @@ -121,11 +123,7 @@ sub NewSession($$$) my $Id; while (defined($Existing)) { - $Id = ""; - foreach my $i (1..8) - { - $Id .= sprintf("%lx", int(rand(2 ** 16))); - } + $Id = Bytes::Random::Secure::random_bytes_hex(16); $Existing = $self->GetItem($Id); } $Session->Id($Id);
-
Francois Gouget