Something is stepping on something

In article <bob-EABE57.17554212022008.RemoveThis@news.verizon.net>,
Robert Peirce <bob.RemoveThis@peirce-family.com.invalid> wrote:

> I have the following definitions in a header file:
>
> /*
> Declarations for database variables
> */
>
> char t[TL]; /* ticker */
> char n[SN]; /* name */
> int g1,g2,g3; /* group codes */
>
> TL is defined as 9 and SN as 31 in another header file.
>
>
> They are used in the following code segment:
>
> fgets(s1, BUF, sd);

What are those 3 things defined as?

> strcpy(t, s1);

What happens if the string at s1 is longer than 8 bytes?

> t[TL-1] = '\0';
> printf ("Ticker = %s.\n", t); // This is correct
> printf ("Something happens between here . . .\n");
>
> strcpy(n,s1+TL); // This is trashing t

What do you expect to find 9 bytes after the start of s1? What happens
if that's longer than 30 characters?

> printf ("And here . . .\n");
> printf ("Ticker = %s.\n", t); // This is trash
> n[SN-1] = '\0';
>
> This code works fine under Uwin on a PC. The module compiles with no
> errors or warnings on my Intel based MacBook Pro. Something seems to be
> getting stepped on but I can't figure out how.
>
> I think I am missing something obvious. Can anybody help?

As a tip, I really recommend using more descriptive variable names.
Terseness is really not a virtue.

Also it's an excellent idea to get into the habit of initializing
variables as the point of declaration.
 >> Stay informed about: Something is stepping on something 

In article <bob-EABE57.17554212022008 DeleteThis @news.verizon.net>,
Robert Peirce <bob DeleteThis @peirce-family.com.invalid> wrote:

> strcpy(t, s1);

Never use strcpy. See:
<http://developer.apple.com/documentation/Security/Conceptual/SecureCodin
gGuide/Articles/BufferOverflows.html#//apple_ref/doc/uid/TP40002577-SW10>
 >> Stay informed about: Something is stepping on something 

In article
<reinder-9A82C7.00190413022008 DeleteThis @fl-69-69-134-194.sta.embarqhsd.net>,
Reinder Verlinde <reinder DeleteThis @verlinde.invalid> wrote:

> In article <bob-EABE57.17554212022008 DeleteThis @news.verizon.net>,
> Robert Peirce <bob DeleteThis @peirce-family.com.invalid> wrote:
>
> > I have the following definitions in a header file:
> >
> > /*
> > Declarations for database variables
> > */
> >
> > char t[TL]; /* ticker */
> > char n[SN]; /* name */
> > int g1,g2,g3; /* group codes */
> >
> > TL is defined as 9 and SN as 31 in another header file.
> >
> >
> > They are used in the following code segment:
> >
> > fgets(s1, BUF, sd);
> >
> > strcpy(t, s1);
> > t[TL-1] = '\0';
> > printf ("Ticker = %s.\n", t); // This is correct
> > printf ("Something happens between here . . .\n");
> >
> > strcpy(n,s1+TL); // This is trashing t
> > printf ("And here . . .\n");
> > printf ("Ticker = %s.\n", t); // This is trash
> > n[SN-1] = '\0';
> >
> > This code works fine under Uwin on a PC. The module compiles with no
> > errors or warnings on my Intel based MacBook Pro. Something seems to be
> > getting stepped on but I can't figure out how.
> >
> > I think I am missing something obvious. Can anybody help?
>
> 1. you do not describe all variables: what is BUF? sd? s1?

BUF is 1024.
sd is the current 160 character line from the s.d file and it is copied
into s1, which is of size BUF.

> 2. I find it strange that you can figure out that "strcpy(n,s1+TL)" is
> the problematic line, but can not find out why. If you use a debugger,
> or even if you just insert print statements to dump all pointer
> values and their contents, it should not be too hard to find out what
> is wrong.

Actually, that is exactly how I found what line was causing the problem,
but I cannot figure out why. s1+TL starts at the name ('n' is a
security name and 't' is a ticker symbol), as it should, and terminates
with a new line as it should.

T and n, which are defined in a header file, are used in a number of
other programs which do the same sort of thing but work as expected.

I have tried to use gdb, but I am having problems understanding how to
set it up properly, so I resorted to print statements.

> 3. I tend to evade programming in 70's C, so I may be wrong, but I
> think that the supposedly zero-terminated argument "s1+TL" to that
> strcpy call may be of arbitrary length if the call "fgets(s1, BUF,
> sd)" read less than TL-1 characters.

I wrote this stuff many years ago and I have ported it to several
different platforms, including a NeXT. It currently runs on a PC under
Uwin. Usually I have no problems with it. Print statements I used
earlier in tracking this down show that sd has all 160 characters. It
it pretty clear that t is getting stepped on by strcpy but I cannot
figure out how or why.

--
Robert B. Peirce, Venetia, PA 724-941-6883
bob AT peirce-family.com [Mac]
rbp AT cooksonpeirce.com [Office]
 >> Stay informed about: Something is stepping on something 

In article <bob-EABE57.17554212022008.DeleteThis@news.verizon.net>,
Robert Peirce <bob.DeleteThis@peirce-family.com.invalid> wrote:

> I have the following definitions in a header file:
>
> /*
> Declarations for database variables
> */
>
> char t[TL]; /* ticker */
> char n[SN]; /* name */
> int g1,g2,g3; /* group codes */
>
> TL is defined as 9 and SN as 31 in another header file.
>
>
> They are used in the following code segment:
>
> fgets(s1, BUF, sd);
>
> strcpy(t, s1);
> t[TL-1] = '\0';
> printf ("Ticker = %s.\n", t); // This is correct
> printf ("Something happens between here . . .\n");
>
> strcpy(n,s1+TL); // This is trashing t
> printf ("And here . . .\n");
> printf ("Ticker = %s.\n", t); // This is trash
> n[SN-1] = '\0';
>
> This code works fine under Uwin on a PC. The module compiles with no
> errors or warnings on my Intel based MacBook Pro. Something seems to be
> getting stepped on but I can't figure out how.
>
> I think I am missing something obvious. Can anybody help?

I just thought of something you Apple experts might know. This has
always worked before because t came in memory before n. I am copying
s1+TL to n, which is bout 150 characters into a 31 character string. It
is going to trash anything that follows it in memory, which has never
been a problem because n always followed t. While this is probably a
really dumb assumption, it has worked for 20 years. However, does that
actually happen in this case? If t follows n in memory, it would
explain why t is getting trashed. It wouldn't quite explain exactly
what is getting into t, but it would be a start.

--
Robert B. Peirce, Venetia, PA 724-941-6883
bob AT peirce-family.com [Mac]
rbp AT cooksonpeirce.com [Office]
 >> Stay informed about: Something is stepping on something 

In article <bob-A2D57D.21410012022008.RemoveThis@news.verizon.net>,
Robert Peirce <bob.RemoveThis@peirce-family.com.invalid> wrote:

> I just thought of something you Apple experts might know. This has
> always worked before because t came in memory before n. I am copying
> s1+TL to n, which is bout 150 characters into a 31 character string. It
> is going to trash anything that follows it in memory, which has never
> been a problem because n always followed t. While this is probably a
> really dumb assumption, it has worked for 20 years. However, does that
> actually happen in this case? If t follows n in memory, it would
> explain why t is getting trashed. It wouldn't quite explain exactly
> what is getting into t, but it would be a start.

Okay, for the heck of it, I tried this code:

strcpy(t, s1);
t[TL-1] = '\0';

strcpy(s2,s1+TL);
s2[SN-1] = '\0';
strcpy(n, s2);
printf ("Ticker = %s.\n", t);
printf ("Name = %s.\n",n);

and it worked. Apparently, t follows n in memory on the Mac and n was
trashing t.

Thanks, everybody, for the tips.

--
Robert B. Peirce, Venetia, PA 724-941-6883
bob AT peirce-family.com [Mac]
rbp AT cooksonpeirce.com [Office]
 >> Stay informed about: Something is stepping on something 

Robert Peirce wrote:
> In article <bob-A2D57D.21410012022008.TakeThisOut@news.verizon.net>,
> Robert Peirce <bob.TakeThisOut@peirce-family.com.invalid> wrote:
>
>> I just thought of something you Apple experts might know. This has
>> always worked before because t came in memory before n. I am copying
>> s1+TL to n, which is bout 150 characters into a 31 character string. It
>> is going to trash anything that follows it in memory, which has never
>> been a problem because n always followed t. While this is probably a
>> really dumb assumption, it has worked for 20 years. However, does that
>> actually happen in this case? If t follows n in memory, it would
>> explain why t is getting trashed. It wouldn't quite explain exactly
>> what is getting into t, but it would be a start.
>
> Okay, for the heck of it, I tried this code:
>
> strcpy(t, s1);
> t[TL-1] = '\0';
>
> strcpy(s2,s1+TL);
> s2[SN-1] = '\0';
> strcpy(n, s2);
> printf ("Ticker = %s.\n", t);
> printf ("Name = %s.\n",n);
>
> and it worked. Apparently, t follows n in memory on the Mac and n was
> trashing t.
>
> Thanks, everybody, for the tips.
>

You really shouldn't rely on this kind of behaviour. Better to write
robust code and take steps to ensure that you never overrun buffers.

Paul
 >> Stay informed about: Something is stepping on something 

In article <61fssiF1ukgf3U2 RemoveThis @mid.individual.net>,
Paul Russell <prussell RemoveThis @sonic.net> wrote:

> You really shouldn't rely on this kind of behaviour. Better to write
> robust code and take steps to ensure that you never overrun buffers.

Folks, please give me a break. This stuff is legacy software, written
over 30 years ago and changed only enough to port it to new machines.
In this case I did make that change and the code works properly

Although I graduated from Carnegie Tech (CMU) in 1964 with a degree in
Electrical Engineering and Computer Science, the CS was primitive. In
fact, the university didn't even create an official CS department until
1965. I couldn't touch any of you who have graduated in the last few
years.

I started on a SouthWest Tecnical Products (Sweat-Pack) with 8"
floppies and a primitive DOS. 8K of RAM was more than enough for any
sensible person!

I was reading about Unix, taking some courses and getting very
enthusiastic when Altos came out with an M6800 computer running an
early version of Unix, if I remember correctly. This saved my the
expense of acquiring a DEC or Perkin-Elmer mini (remember those??).
This was sometime in the late 1970s.

The Altos had a C compiler and I started to write code to support my
work in the investment business. The goal was to cobble together
something that worked, not to be particularly elegant. The early C
compilers were very lenient and you could get away with murder, which I
did. At least I created man pages for all my programs!

I went from there to an Altos 68020, running Unix System III. Then to
a NeXT and most recently to a PC running Uwin. Uwin uses ksh and its
C compiler is actually a front-end to Microsoft's C compiler.

Through all of this I was usually able simply to recompile the old
code for the new architecture, sometimes making a few changes.
Consequently, it hasn't changed much in over 30 years. C is a
remarkably robust language.

Since my business is managing money for people, not software design, I
have not moved ahead. I never learned C++ or ObjectiveC, because
everything I needed was already written in C. For the same reason, I
never learned Perl, Python, Ruby or any of the languages that have come
along since awk, which has always served my needs. The one concession
I have made is to pick up a smattering of ksh.

By most of your standards I am an old foggy living in the last
century. As a software engineer that is true, but that is not my
business. My business is to manage money and the computer just
supports that task. I happen to like coding, which is why I ended up
doing it, but I always had to make that secondary to my primary role.
By your standards I am way out of my depth, which is why I turn to you
when I run into something I don't understand.

--
Robert B. Peirce, Venetia, PA 724-941-6883
bob AT peirce-family.com [Mac]
rbp AT cooksonpeirce.com [Office]
 >> Stay informed about: Something is stepping on something 

Robert Peirce wrote:
> In article <61fssiF1ukgf3U2.TakeThisOut@mid.individual.net>,
> Paul Russell <prussell.TakeThisOut@sonic.net> wrote:
>
>> You really shouldn't rely on this kind of behaviour. Better to write
>> robust code and take steps to ensure that you never overrun buffers.
>
> Folks, please give me a break. This stuff is legacy software, written
> over 30 years ago and changed only enough to port it to new machines.
> In this case I did make that change and the code works properly
>
> [resume snipped]
>

It wasn't meant to be a criticism, just a suggestion. You should never
assume anything about the layout of variables in memory - it can change
not only between different machines and different compilers but also
between different builds (e.g. if you change compiler switches). Rather
than repeatedly "fixing" these problems on an ad hoc basis you could
just invest a little time making the code more robust - it would
probably be a net win in the long run.

Paul
 >> Stay informed about: Something is stepping on something 

In article <61j175F1svdtbU1.DeleteThis@mid.individual.net>,
Paul Russell <prussell.DeleteThis@sonic.net> wrote:

> you could
> just invest a little time making the code more robust - it would
> probably be a net win in the long run.

This is the fourth time I have ported the software in 30+ years. I am
67 and will probably never have to port it again :-). Nevertheless, the
time required to re-write all the code versus the time required to port
it every 5-10 years makes the minor fixes worthwhile.

The fact is, I had to port 10 C programs (and 27 ksh scripts), of which
9 compiled and ran with no problems. (All the ksh scripts required me
to add '#!/bin/ksh' in the first line because OS X did not default to
ksh, but after that, they ran fine as well). The tenth used the same
module one of the 9 used and had a problem only because I added code for
it a number of years ago in a very quick and unusually dirty way.

Bottom line is most of the code was pretty clean going in and I have
re-done this problem in a proper fashion to store character strings of
the correct size.

I am sure there are a number of other hidden gotchas buried in this
code, but thankfully, I probably won't have to worry about them.

--
Robert B. Peirce, Venetia, PA 724-941-6883
bob AT peirce-family.com [Mac]
rbp AT cooksonpeirce.com [Office]
 >> Stay informed about: Something is stepping on something 

In article <61j175F1svdtbU1 RemoveThis @mid.individual.net>,
Paul Russell <prussell RemoveThis @sonic.net> wrote:

> Robert Peirce wrote:
> >
> > Folks, please give me a break. This stuff is legacy software, written
> > over 30 years ago and changed only enough to port it to new machines.
> > In this case I did make that change and the code works properly
> >
> > [resume snipped]
> >
>
> It wasn't meant to be a criticism, just a suggestion. You should never
> assume anything about the layout of variables in memory - it can change
> not only between different machines and different compilers but also
> between different builds (e.g. if you change compiler switches). Rather
> than repeatedly "fixing" these problems on an ad hoc basis you could
> just invest a little time making the code more robust - it would
> probably be a net win in the long run.

I second both the "it is just a suggestion", and the "still, I repeat
that suggestion" parts. This does not have to mean staring for hours at
the source. There are tools that can help here, such as clint
(<http://sourceforge.net/projects/clint/>) and/or valgrind
(<http://valgrind.org/>) variant. The things these tools report might
even help you solve some "it crashes once in a while", "the results look
odd once in a while" or "it does not work when I enable compiler
optimization" issues.

Reinder
 >> Stay informed about: Something is stepping on something