[KinoSearch] Revision 3591 t/029-charbuf failing with buffer overflow

[Marvin Humphrey]

On Jul 8, 2008, at 6:16 PM, Henry wrote:

> Guten Morgen allerseits!

Hi, sorry about the delay in responding.  There were problems at the  
facility where the rectangular.com box is colocated, and after it went  
down last Wednesday it took a while to get it back in service.

> Revision 3591 is failing one of it's tests:
>
> t/029-charbuf....................ok 1/34*** buffer overflow detected  
> ***:

This overflow happened to occur in a sloppily coded test case rather  
than the CB_VCatF method itself.  The problem was that I added more  
characters to the test case but forgot to change the buffer size where  
the "wanted" string was being sprintf'd, and we wound up with a buf of  
20 chars when we needed 21 to hold the full string including the  
terminating NULL:

@@ -135,7 +147,7 @@
vcatf_f(VArray *tests)
{
     CharBuf *wanted;
-    char buf[20];
+    char buf[64];
     float num = 1.3f;
     CharBuf *got = get_cb("foo ");
     sprintf(buf, "foo bar %f baz", num);


Curiously, Valgrind didn't report an error on my system, though in  
theory it probably should have.  In any case, r3592 should resolve the  
issue.

Nevertheless, it's worth reviewing one aspect of CB_VCatF anyway.   
CB_VCatF is meant to be a crude and slow but safe formatted string  
concatenator -- a poor relation of vsnprintf or the perlapi function  
sv_vcatpvfn.  Its primary use is error messages.  CB_VCatF does one  
fancy thing that printf and friends don't: "%o" takes a Boilerplater  
object and concatentates the output of the object's To_String()  
method.  However, at present CB_VCatF handles no formatting modifiers  
at all: "%f" only, not "%0.2f" and such.  That makes it relatively  
easy to calculate how much space CB_VCatF needs in comparison to  
something hard like a full implementation of snprintf.  So, the  
question is... Are there any systems on which a sprintf'd float with  
no modifiers ever requires more than the buffer size used in  
kino_CB_vcatf(), 64 bytes?

Since your system failed when mine didn't, try this C app and tell me  
what you get:

   #include <stdio.h>
   #include <float.h>
   int main () {
     char buf[128];
     unsigned len = sprintf(buf, "%f", FLT_MAX);
     printf("%s\n", buf);
     printf("Bytes: %u\n", len);
     return 0;
   }

This is what I see:

   340282346638528859811704183484516925440.000000
   Bytes: 46

For reference, here's a relevant chunk taken from Perl_sv_vcatpvfn, in  
sv.c

     /* Times 4: a decimal digit takes more than 3 binary digits.
      * NV_DIG: mantissa takes than many decimal digits.
      * Plus 32: Playing safe. */
     char ebuf[IV_DIG * 4 + NV_DIG + 32];
     /* large enough for "%#.#f" --chip */
     /* what about long double NVs? --jhi */

We don't need quite as much as Perl does, though, because CB_VCatF  
doesn't accept modifiers.  No doubles / long doubles, either.

Marvin Humphrey
Rectangular Research
http://www.rectangular.com/


_______________________________________________
KinoSearch mailing list
KinoSearch at rectangular.com
http://www.rectangular.com/mailman/listinfo/kinosearch