Re: strncpy is not a safe version of strcpy

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tomas Vondra <tv(at)fuzzy(dot)cz>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: strncpy is not a safe version of strcpy
Date: 2014-08-13 10:26:01
Message-ID: CAApHDvp6YM9tSBCLZOrA-P5oAWjoN-dDU7200O7wERsiG=9EYA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Aug 13, 2014 at 3:19 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:

> On Sat, Nov 16, 2013 at 12:53:10PM +1300, David Rowley wrote:
> > I went on a bit of a strncpy cleanup rampage this morning and ended up
> > finding quite a few places where strncpy is used wrongly.
> > I'm not quite sure if I have got them all in this patch, but I' think
> I've
> > got the obvious ones at least.
> >
> > For the hash_search in jsconfuncs.c after thinking about it a bit more...
> > Can we not just pass the attname without making a copy of it? I see
> keyPtr
> > in hash_search is const void * so it shouldn't get modified in there. I
> > can't quite see the reason for making the copy.
>
> +1 for the goal of this patch. Another commit took care of your
> jsonfuncs.c
> concerns, and the patch for CVE-2014-0065 fixed several of the others.
> Plenty
> remain, though.
>
>
Thanks for taking interest in this.
I had a quick look at the usages of strncpy in master tonight and I've
really just picked out the obviously broken ones for now. The other ones,
on first look, either look safe, or require some more analysis to see
what's actually done with the string.

I think this is likely best tackled in small increments anyway.

Does anyone disagree with the 2 changes in the attached?

Regards

David Rowley

Attachment Content-Type Size
strncpy_fixes_pass1.patch application/octet-stream 981 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2014-08-13 10:31:37 Re: 9.5: Memory-bounded HashAgg
Previous Message Pavel Stehule 2014-08-13 09:39:33 Re: proposal for 9.5: monitoring lock time for slow queries