Mailing List Archive

Wait a minute here...

On Wed, Jan 19, 2000 at 02:39:18AM -0500, David Rankin wrote:
> While rototilling packet.c, I did some looking at cipher_encrypt in
> cipher.c. It ends up that for SSH_CIPHER_NONE in cipher_encrypt, it
> uses memcpy. However, it also appears that dest and src can be equal
> in cipher_encrypt.

> On most sane libc implementations, memcpy == memmove. However, ANSI C
> makes no such guarantee, and some implementations out there are bound
> to try to optimize memcpy eventually.

Check me on this...

The danger in those routines has always been overlaping regions.
If the source and destination do not overlap, you are safe, of course.
If the source and destination are the same, you are also safe since you
are setting memory back to its original value. Length limited functions
such as memcpy were never in danger of runaway conditions like strcpy was,
but they could have unpredictable results in memory if the source and
destination where overlaping and non-congruent (not identical).

If the src and dest can be equal, I don't see the problem. If they
can be non-equal but overlapping, then we have a potential problem.

> Therefore, I've hacked out the following patch to change what may be
> "dangerous" memcpy's to memmove. Take a look and see what you think.

I didn't look at it real close but it didn't look like it did
any harm. Is it going to cause any platform support and compatibility
issues?

> Thanks,
> David

> --
> David W. Rankin, Jr. Husband, Father, and UNIX Sysadmin.
> Email: drankin@bohemians.lexington.ky.us Address/Phone Number: Ask me.
> "It is no great thing to be humble when you are brought low; but to be humble
> when you are praised is a great and rare accomplishment." St. Bernard

Mike
--
Michael H. Warfield | (770) 985-6132 | mhw@WittsEnd.com
(The Mad Wizard) | (770) 331-2437 | http://www.wittsend.com/mhw/
NIC whois: MHW9 | An optimist believes we live in the best of all
PGP Key: 0xDF1DD471 | possible worlds. A pessimist is sure of it!

On Wed, Jan 19, 2000 at 09:47:29AM -0500, Michael H. Warfield wrote:
> Wait a minute here...

> On Wed, Jan 19, 2000 at 02:39:18AM -0500, David Rankin wrote:
...
> > On most sane libc implementations, memcpy == memmove. However, ANSI C
> > makes no such guarantee, and some implementations out there are bound
> > to try to optimize memcpy eventually.

> Check me on this...
>
> The danger in those routines has always been overlaping regions.
> If the source and destination do not overlap, you are safe, of course.
> If the source and destination are the same, you are also safe since you
> are setting memory back to its original value. Length limited functions
> such as memcpy were never in danger of runaway conditions like strcpy was,
> but they could have unpredictable results in memory if the source and
> destination where overlaping and non-congruent (not identical).
>
> If the src and dest can be equal, I don't see the problem. If they
> can be non-equal but overlapping, then we have a potential problem.

Thank you. My ability to explain things always deteriotates when up that
late. (The baby's been liking her 2AM feeds this week...)

Basically, I'm a paranoid programmer. I like to see subroutines be
paranoid, since you never know who will be calling you later. memmove
isn't that much more expensive than memcpy, and it keeps someone from
(knowingly or unknowingly) giving the routines bad data.

Also, to be horribly pedantic, memcpy isn't guaranteed to work when
src == dest. Any sane implementation will work, but the standard would
call that "undefined behaviour".

> > Therefore, I've hacked out the following patch to change what may be
> > "dangerous" memcpy's to memmove. Take a look and see what you think.

> I didn't look at it real close but it didn't look like it did
> any harm. Is it going to cause any platform support and compatibility
> issues?

We're already assuming full ANSI C support, and memmove is definitely
defined in the ANSI C language specs. That said, I can see where there
might be platforms with buggy implementations, but these should be
extremely rare these days.

David

--
David W. Rankin, Jr. Husband, Father, and UNIX Sysadmin.
Email: drankin@bohemians.lexington.ky.us Address/Phone Number: Ask me.
"It is no great thing to be humble when you are brought low; but to be humble
when you are praised is a great and rare accomplishment." St. Bernard