SecFix for databases/firebird, please review

[Alexander Leidinger]

On Wed, 27 Aug 2003 10:10:36 +0200
Pawel Jakub Dawidek <nick at garage.freebsd.pl> wrote:

> On Mon, Aug 18, 2003 at 11:59:28AM +0200, Alexander Leidinger wrote:
> +> Thanks for the review. I've updated
> +> http://www.leidinger.net/FreeBSD/firebird-1.0.2-secfix.tar.bz2 (modulo
> +> Chris' work in progress). I'm looking forward to the next round. :-)
> 
> IMHO there are still problems with strncat(3).
> 
> If you use something like that:
> 
> 	strncat(buf, string, sizeof(buf) - 1);
> 
> why not just use:
> 
> 	strncpy(buf, string, sizeof(buf) - 1);

Because behavior of strncpy != behavior of strncat. And I want to
preserve as much as possible from the original code.

> because correct form is:
> 
> 	strncat(buf, string, sizeof(buf) - strlen(buf) - 1);

Where did I missed this? Yes, I don't use strlen(destination), I use
strlen(previous_source) instead, but the result is the same.

> There is also syntax error here:
> 
> 	strncat (ib_prefix_msg_val, MAXPATHLEN, ib_prefix_msg);

Oops. Fixed locally.

> You also still don't add:
> 
> 	buf[sizeof(buf) - 1] = '\0';
> 
> after all strncat(3)s.

Hmmm... yes, I see the problem...

> This was in first patch:)

It's the largest one...

> IMHO if you want to keep portability, just add some BSD-licensed strlcat(3)
> and strlcpy(3) implementations to firebird's code and use it, because
> strncat(3) is really fucked up.

Or just use strl* in the port und wait for firebird 1.5... Chris, what
do you think?

Bye,
Alexander.

-- 
   If Bill Gates had a dime for every time a Windows box crashed...
                ...Oh, wait a minute, he already does.

http://www.Leidinger.net                       Alexander @ Leidinger.net
  GPG fingerprint = C518 BC70 E67F 143F BE91  3365 79E2 9C60 B006 3FE7