Re: buffer overflow detected in bcrelay

--===============3563181898972290399==
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="LfQcPIWFRhGivmDw"
Content-Disposition: inline


--LfQcPIWFRhGivmDw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

James Cameron wrote...

> The code was certainly unsafe, using sprintf like that.
> I've changed it to use snprintf, please review and comment.

This here ...

  for (...)
    {
        (...)
        logstr_cntr =3D snprintf(logstr,
                               MAX_LOG_INTERFACES - strlen(log_interfaces),
                               "%s ", iflistLogIToString(&(iflist[i]),
                                                         i, &(cur_ifsnr[i])=
));
        logstr +=3D logstr_cntr;
    }

=2E.. will not behave as expected if snprintf exhausts the buffer.
Beware, snprintf returns the number of octets it *wanted* to print,
not the number *actually* printed. As a result, logstr will point
beyond the buffer length, resulting in another buffer overflow in the
next loop.

Dropping logstr_cntr and doing "logstr +=3D strlen (logstr);" should do
the right thing.

Some nitpicking:

This change
+    MAX_LOG_INTERFACES - strlen(log_interfaces),
-    MAX_LOG_INTERFACES - (logstr-log_interfaces),
should result in faster execution but it probably doesn't matter.

Additionally, I'd always write
    #define MAX_LOG_RELAYED MAX_IFLOGTOSTR*MAXIF+81
as
    #define MAX_LOG_RELAYED (MAX_IFLOGTOSTR*MAXIF+81)
since else any usage like in
    if (MAX_LOG_RELAYED*2 > limit) ...
might create some surprise. As far as I can see, such operations do
not exist in bcrelay.c at the moment but cowardly as I am ...

Finally, care to introduce a constant here?
    static char interfaces[32];
Plain numbers always give me a bad feeling.

But it's more than a decade since I've stopped programming in C, so
my knowledge might be little rusty.

    Christoph
--=20

--LfQcPIWFRhGivmDw
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iQIcBAEBCgAGBQJUPwDMAAoJEMQsWOtZFJL9HZYP/3B2wXaBDR1gGXt1Ii0c4AGd
fC60OW5ed+L67y8jIoqFsdzETOHE21AYYGnAMeaRFj8cFgbOjmNxm+8RpSga0a4E
jCXXbRFZzWUbDo8UahSU/tdhTdyubpBCUXOHGwXSfi3uHV7D9TxqiZ7VYftsHqyr
H1Xrqp3wjaKXTV5ZNYNN4ZYVbxwbqI5et0JVL0xwO6X1L8iLEdagwBvd5WXMoctW
7kjE2NemIrTfSGuQ2JLaYKM6+ZideLw8G8IlX5nQWuEftCOB6/SIkUd4ZK+iEoBD
uzkMlN5qvKVDWZOb0kD3eGutBwJV4VE9gGV2oMGY1cofrF5A5jC7Pdml+xMpaCHl
+eQSvQp680MrxOiVoPJOy7RVFSRwltMF8IUGQhPeF5wC1Kt/UfBgbOV0lYkN9Uog
f+bxuQcUZ2hUtfpj6/CqbOujbUQfGXdPp/l1gJrShAuufUnetwpBsKkZdK0+NJLU
aWHYe+8EejNwkghYRDU2qiYIva8ZoBWoPkhNM0StRfvRFIQbWgFGWlLS2vu7Fzmj
yqQH1wlwIaq0wkLSetRwZqN7amkIwa8KC0U4eeFLc5NZG0IO6wS1PkcSax8XraGw
nt4a/KeVvar/q7j5+OZPZG2+IUTUI2dy57w4ligigib2GHs0lpyCviqPSRwNMo4z
fpJ9wg4rKRm2Yd6ihQlK
=papO
-----END PGP SIGNATURE-----

--LfQcPIWFRhGivmDw--


--===============3563181898972290399==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
--===============3563181898972290399==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Poptop-server mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/poptop-server

--===============3563181898972290399==--