GH-1076: Fix bro_prng() implementation

The intermediate result of the PRNG used unsigned storage, preventing
the ( result < 0 ) branch from ever being evaluated.  This could cause
return values to exceed the modulus as well as RAND_MAX.

One interesting effect of this is potential for the rand() BIF to
return values outside the requested maximum limit.

Another interesting effect of this is that a PacketFilter may start
randomly dropping packets even if it was not configured for
random-packet-drops.
This commit is contained in:
Jon Siwek 2020-07-21 23:19:24 -07:00
parent dba764386b
commit 0f4eb9af02
3 changed files with 30 additions and 13 deletions

View file

@ -1186,18 +1186,22 @@ bool have_random_seed()
unsigned int bro_prng(unsigned int state) unsigned int bro_prng(unsigned int state)
{ {
// Use our own simple linear congruence PRNG to make sure we are // Use our own simple linear congruence PRNG to make sure we are
// predictable across platforms. // predictable across platforms. (Lehmer RNG, Schrage's method)
static const long int m = 2147483647; constexpr uint32_t m = 2147483647;
static const long int a = 16807; constexpr uint32_t a = 16807;
const long int q = m / a; constexpr uint32_t q = m / a;
const long int r = m % a; constexpr uint32_t r = m % a;
state = a * ( state % q ) - r * ( state / q ); uint32_t rem = state % q;
uint32_t div = state / q;
int32_t s = a * rem;
int32_t t = r * div;
int32_t res = s - t;
if ( state <= 0 ) if ( res < 0 )
state += m; res += m;
return state; return res;
} }
long int bro_random() long int bro_random()

View file

@ -3,13 +3,12 @@
#empty_field (empty) #empty_field (empty)
#unset_field - #unset_field -
#path conn #path conn
#open 2019-07-31-22-25-32 #open 2020-07-22-05-02-04
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents
#types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] #types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string]
1254722767.492060 CHhAvVGS1DHFjwGM9 10.10.1.4 56166 10.10.1.1 53 udp dns 0.034025 34 100 SF - - 0 Dd 1 62 1 128 - 1254722767.492060 CHhAvVGS1DHFjwGM9 10.10.1.4 56166 10.10.1.1 53 udp dns 0.034025 34 100 SF - - 0 Dd 1 62 1 128 -
1254722776.690444 C4J4Th3PJpwUYZZ6gc 10.10.1.20 138 10.10.1.255 138 udp - - - - S0 - - 0 D 1 229 0 0 - 1254722776.690444 C4J4Th3PJpwUYZZ6gc 10.10.1.20 138 10.10.1.255 138 udp - - - - S0 - - 0 D 1 229 0 0 -
1254722767.529046 ClEkJM2Vm5giqnMf4h 10.10.1.4 1470 74.53.140.153 25 tcp - 0.346950 0 0 S1 - - 0 Sh 1 48 1 48 - 1254722767.529046 ClEkJM2Vm5giqnMf4h 10.10.1.4 1470 74.53.140.153 25 tcp - 0.346950 0 0 S1 - - 0 Sh 1 48 1 48 -
1437831776.764391 CtPZjS20MLrsMUOJi2 192.168.133.100 49285 66.196.121.26 5050 tcp - 0.343008 41 0 OTH - - 0 Da 1 93 1 52 - 1437831776.764391 CtPZjS20MLrsMUOJi2 192.168.133.100 49285 66.196.121.26 5050 tcp - 0.343008 41 0 OTH - - 0 Da 1 93 1 52 -
1437831798.533765 CmES5u32sYpV7JYN 192.168.133.100 49336 74.125.71.189 443 tcp - - - - OTH - - 0 A 1 52 0 0 - 1437831787.856895 CUM0KZ3MLUfNB0cl11 192.168.133.100 49648 192.168.133.102 25 tcp - 0.004707 0 0 S1 - - 0 Sh 1 64 1 60 -
1437831787.856895 CUM0KZ3MLUfNB0cl11 192.168.133.100 49648 192.168.133.102 25 tcp - 0.048043 162 154 S1 - - 154 ShDgA 3 192 1 60 - #close 2020-07-22-05-02-04
#close 2019-07-31-22-25-32

View file

@ -32,6 +32,20 @@ event zeek_init()
print d; print d;
print e; print e;
print f; print f;
local i = 0;
local max = 3;
while ( i < 100 )
{
local rn = rand(max);
if ( rn >= max )
print fmt("ERROR: rand returned value greater than %s: %s",
max, rn);
i += 1;
}
} }
event zeek_init() &priority=-10 event zeek_init() &priority=-10