[Serusers] Crash in branch_builder() with probability 1/RAND_MAX

[Jiri Kuthan]

At 01:28 AM 4/12/2003, Jiri Kuthan wrote:
>Jonathan,
>
>thanks for reporting the bug. We encountered it some time ago (it is fixed
>the same on CVS the way you propose, by testing char_v) but I didn't update
>INSTALL -- shame on me and sorry about that. It's now refered from there,

s/INSTALL/ISSUES

>including a patch.
>
>-Jiri
>
>At 10:06 PM 4/10/2003, Jonathan Lennox wrote:
>>I've discovered a bug in SER 0.8.10 which can segfault the server with a
>>dereference of NULL, one time in RAND_MAX.
>>
>>In the function branch_builder() in msg_translator.c, if both the parameter
>>'label' and the parameter 'char_v' are 0, SER will crash.  This is because
>>the code assumes that if label is 0, char_v is non-NULL, and so will attempt
>>to call memcpy() with char_v as the source.
>>
>>When branch_builder is invoked by the tm module, however, the label
>>parameter comes from a random value assigned by h_table.c.  This value is
>>generated by rand().  As such, its value can legitimately be 0, which will
>>happen, on average, one time in RAND_MAX.
>>
>>On Linux, RAND_MAX is 2^31, so this crash is very unlikely.  However, on
>>Solaris (where I'm doing some testing), RAND_MAX is 2^15, so this crash is
>>reasonably common for a server under heavy load.  However, this is a "valid"
>>crash in either case; this isn't just a portability issue.
>>
>>(Note that RAND_MAX == 2^15 being less than TABLE_ENTRIES == 2^16 can also
>>cause problems, according to a comment in h_table.c, though I believe only
>>ones of efficiency, not correctness.)
>>
>>The patch below works around the problem in the simplest possible way,
>>though it isn't a correct fix.  I suspect the proper solution would be a) to
>>reverse the logic of branch_builder() to test char_v for NULL, rather than
>>label for non-0; and b) to check with the preprocessor if RAND_MAX is less
>>than TABLE_ENTRIES, and if so, use random() rather than rand() in
>>modules/tm/h_table.c.
>>
>>-- 
>>Jonathan Lennox
>>lennox at cs.columbia.edu
>>
>>--- ser-0.8.10.orig/msg_translator.c    Mon Oct 21 15:21:50 2002
>>+++ ser-0.8.10/msg_translator.c Wed Apr  9 15:31:47 2003
>>@@ -813,6 +813,12 @@
>>                begin++; size--;
>>        } else return 0;
>> 
>>+        if (!label && !char_v) {
>>+               LOG(L_ERR, "ERROR: branch_builder: both label and char_v "
>>+                    "are 0\n");
>>+               return 0;
>>+        }
>>+
>>        /* label is set -- use it ... */
>>        if (label) {
>>                if (int2reverse_hex( &begin, &size, label )==-1)
>>_______________________________________________
>>Serusers mailing list
>>serusers at lists.iptel.org
>>http://lists.iptel.org/mailman/listinfo/serusers 
>
>--
>Jiri Kuthan            http://iptel.org/~jiri/ 
>
>_______________________________________________
>Serusers mailing list
>serusers at lists.iptel.org
>http://lists.iptel.org/mailman/listinfo/serusers 

--
Jiri Kuthan            http://iptel.org/~jiri/