C++ code dore dumping, not sure where the problem lies

[draw...@gmail.com]

Hi

I seem to not figure out whats going on with a piece of C++ code that I had written a while back, generally the program checks a logged in user's credentials to decide is he/she is authorized to access certain linux servers. The final bit in this code creates an SSH connection object. No, its not what you may think, its not using any SSH library, the code just constructs the SSH command line and executes the same using the system() call, it seems to be dumping core for some new servers my colleague has created.

This is the general behaviour, the user issues the command he/she is supposed to and gets the command prompt for the target server, when the user is done with their work and enters exit at the target server's command prompt, the user logs out with the following:

[user@machine xyz ~]$ exit
logout
Segmentation fault (core dumped)


The command that goes into the system() call looks something like this:
/usr/bin/ssh -o \"StrictHostKeyChecking no\" -i /path/to/protected/keys/id_rsa us...@192.168.44.73 -p 5555 2>/dev/null

An this is where its dumping core, well, somewhere here:


sshconn::sshconn(creds &c){
//construct the string
char connstr[200];
char buf[250];
log lg("/path/to/log/file.log");
memset((void*)connstr,'0',200);
strcpy(connstr,"/usr/bin/ssh -o \"StrictHostKeyChecking no\" -i /path/to/protected");
strcat(connstr,c.getAuthStr());
strcat(connstr,"/keys/id_rsa ");
strcat(connstr,c.getUser());
strcat(connstr,"@");
//To continue...with getting hostname and IP
try{
getHostNameIP(c,connstr);
getPortNumber(c,connstr);
}catch(FileCantOpen &e){
e.printMsg();
exit(10);
}catch(FileCantRead &e){
e.printMsg();
exit(11);
}
strcat(connstr," 2>/dev/null");
memset(buf,'\0',sizeof(buf));
sprintf(buf,"User %s connecting with connection string %s",c.getUser(),connstr);
lg.write(buf);
system(connstr);
}

When I use gdb, it says that the segsegv happened at the last curly brace, but that doesn't make sense, it always happens at logout. Any clues what is causing this would be immensely helpful.

[draw...@gmail.com]

Sorry for the typo, I mean core dumping, not "dore" dumping.

[Scott Lurndal]

draw...@gmail.com writes:
>Hi
>
>I seem to not figure out whats going on with a piece of C++ code that I had=
> written a while back, generally the program checks a logged in user's cred=
>entials to decide is he/she is authorized to access certain linux servers. =
>The final bit in this code creates an SSH connection object. No, its not wh=
>at you may think, its not using any SSH library, the code just constructs t=
>he SSH command line and executes the same using the system() call, it seems=

> to be dumping core for some new servers my colleague has created.
>

>This is the general behaviour, the user issues the command he/she is suppos=
>ed to and gets the command prompt for the target server, when the user is d=
>one with their work and enters exit at the target server's command prompt, =

>the user logs out with the following:
>
> [user@machine xyz ~]$ exit
> logout
> Segmentation fault (core dumped)
>
>
>The command that goes into the system() call looks something like this:

> /usr/bin/ssh -o \"StrictHostKeyChecking no\" -i /path/to/protected/keys=

>/id_rsa us...@192.168.44.73 -p 5555 2>/dev/null
>
>An this is where its dumping core, well, somewhere here:
>
>
> sshconn::sshconn(creds &c){
> //construct the string
> char connstr[200];

It's entirely likely that given a FQDN and long username that
you are writing beyond the end of connstr, corrupting the stack,
which would cause the SIGSEGV when returning from the function.

Using std::string instead of a char buffer + strcat would avoid this.

> char buf[250];
> log lg("/path/to/log/file.log");
> memset((void*)connstr,'0',200);

This should never be done. Use sizeof(connstr) instead of hardcoding 200,
or at minimum use the same #define macro for both the declaration and as
the third argument of memset.

I also suspect that you wanted '\0' (or just 0) instead of '0' in the memset.

[Victor Bazarov]

On 8/17/2015 9:53 AM, Scott Lurndal wrote:
> draw...@gmail.com writes:
>> [...]

>> char buf[250];
>> log lg("/path/to/log/file.log");
>> memset((void*)connstr,'0',200);
>
> This should never be done. Use sizeof(connstr) instead of hardcoding 200,
> or at minimum use the same #define macro for both the declaration and as
> the third argument of memset.
>
> I also suspect that you wanted '\0' (or just 0) instead of '0' in the memset.

If 0 is what the real intent was, then simple initialization should suffice:

char buf[200] = {};

V
--
I do not respond to top-posted replies, please don't ask

[Barry Schwarz]

On Mon, 17 Aug 2015 05:15:18 -0700 (PDT), draw...@gmail.com wrote:

<snip>

>The command that goes into the system() call looks something like this:
> /usr/bin/ssh -o \"StrictHostKeyChecking no\" -i /path/to/protected/keys/id_rsa us...@192.168.44.73 -p 5555 2>/dev/null
>
>An this is where its dumping core, well, somewhere here:

<snip>

> strcpy(connstr,"/usr/bin/ssh -o \"StrictHostKeyChecking no\" -i /path/to/protected");
> strcat(connstr,c.getAuthStr());
> strcat(connstr,"/keys/id_rsa ");

....

<snip>

Printing the contents of connstr or at least the length of the string
it contains after each call to strcat might provide some useful
information.

--
Remove del for email

[Richard]

[Please do not mail me a copy of your followup]

sl...@pacbell.net spake the secret code
<yHlAx.65234$MQ3....@fx13.iad> thusly:

>Using std::string instead of a char buffer + strcat would avoid this.

Another perfect example of why you should stop using C-isms in C++.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
The Computer Graphics Museum <http://computergraphicsmuseum.org>
The Terminals Wiki <http://terminals.classiccmp.org>
Legalize Adulthood! (my blog) <http://legalizeadulthood.wordpress.com>

[Scott Lurndal]

legaliz...@mail.xmission.com (Richard) writes:
>[Please do not mail me a copy of your followup]
>
>sl...@pacbell.net spake the secret code
><yHlAx.65234$MQ3....@fx13.iad> thusly:
>
>>Using std::string instead of a char buffer + strcat would avoid this.
>
>Another perfect example of why you should stop using C-isms in C++.

And of course, counter-examples abound. snprintf could have been
used in place of strcat (and would have avoided the buffer overflow).

Even proper use of strncat would have sufficed.

The fact that bad C code is also bad in a C++ program should go without
saying.

[Christopher Pisz]

Given the constant that bad programmers are everywhere, and another that
even good programmers make mistakes, it would be better for them to
first consider solutions where such mistakes can't be made, if the
outcome and efficiency is the same (or with a difference small enough to
be negligible).



--
I have chosen to troll filter/ignore all subthreads containing the
words: "Rick C. Hodgins", "Flibble", and "Islam"
So, I won't be able to see or respond to any such messages
---

[Christian Gollwitzer]

Am 17.08.15 um 14:15 schrieb draw...@gmail.com:

> I seem to not figure out whats going on with a piece of C++ code that
> I had written a while back, generally the program checks a logged in
> user's credentials to decide is he/she is authorized to access
> certain linux servers.

> [...] sigsegv

Use valgrind. Apart from the good advice from the other gentlemen, to
rewrite the code using more C++ features to help avoiding these
problems, a memory checker can probably tell you where the problem
really occurs. Run it in valgrind and fix the first error.

Christian

[Richard]

[Please do not mail me a copy of your followup]

sl...@pacbell.net spake the secret code

<TupAx.89533$VI.4...@fx23.iad> thusly:

>legaliz...@mail.xmission.com (Richard) writes:
>>[Please do not mail me a copy of your followup]
>>
>>sl...@pacbell.net spake the secret code
>><yHlAx.65234$MQ3....@fx13.iad> thusly:
>>
>>>Using std::string instead of a char buffer + strcat would avoid this.
>>
>>Another perfect example of why you should stop using C-isms in C++.
>

>And of course, counter-examples abound. [...]

I'm not really sure how this is a "counter example".

A counter example would be an example of why you *should* use C-isms
in C++.

Should use them as in: gives easier to understand code when reading it
(i.e. appropriate use of abstraction) because code is read more often
than it is written and is less error-prone when using it (particularly
for error handling).

By all means, if you want to write C, then please head on over to
comp.lang.c, contribute to the unix kernel, or whatever. However,
please stop teaching people that writing good C++ means using C-isms.

My personal experience since the mid 90s is the exact opposite:
using C-isms in C++ code creates bugs, reduces maintainability,
reduces readability and prevents understanding because of its inherent
low-level nature and explosion of details. My teams have fewer
errors and less confusing code when they avoid C-isms and stick to C++.
Entire classes of bugs literally disappeared from our code once we stopped
using C style strings, for instance. The original poster's code was
another example of how simply using std::string avoids entire classes
of errors that are too easy to make with C-style strings.

By the way, I am not alone in this opinion nor am I alone in this
multi-decadal cumulative experience with real-world teams. This advice
is echoed by Bjarne Stroustrup in "C++ Programming Language", 4th ed. as
well as recent talks he has given on the nature of C++:

"The Essence of C++"
<https://www.youtube.com/watch?v=86xWVb4XIyE>
"What -- if anything -- have we learned from C++?"
<https://www.youtube.com/watch?v=2egL4y_VpYg>
"Five Popular Myths about C++"
<http://www.stroustrup.com/Myths-final.pdf>

The strength of C++ is that it provides the ability to create abstractions
while still yielding efficient code execution. In many cases the
abstractions are literally free when compared to the low-level detail
oriented code you would need to write in C to achieve the same end result.

[draw...@gmail.com]

On Monday, August 17, 2015 at 5:45:55 PM UTC+5:30, draw...@gmail.com wrote:

Hi

Thanks guys, I have changed my code to use std::string instead of raw character strings and I have rid me of segmentation violations.

Thanks all for pointing the problem areas out for me.

ciao

[Scott Lurndal]

legaliz...@mail.xmission.com (Richard) writes:
>[Please do not mail me a copy of your followup]
>

>I'm not really sure how this is a "counter example".
>
>A counter example would be an example of why you *should* use C-isms
>in C++.

snprintf vs. "<<" and ">>" should be sufficient to show
how the readability (and maintainability) of C++ code is
improved by selective use of C run-time library functions.

You, like Stefan, seem to think that only "pure" C++ is
useful or valid, which is far from the actual truth.

>
>By all means, if you want to write C, then please head on over to
>comp.lang.c, contribute to the unix kernel, or whatever.

Believe that I _have_ contributed to both the unix kernels and
the linux kernels (and even the oracle RDBM core).

>However,
>please stop teaching people that writing good C++ means using C-isms.

However, you have no right to make such a silly demand. There
are many cases where using C run-time library functions and C
idioms in C++ code is perfectly valid. It's perfectly legal use
of the C++ language as well.

>
>By the way, I am not alone in this opinion nor am I alone in this
>multi-decadal cumulative experience with real-world teams. This advice
>is echoed by Bjarne Stroustrup in "C++ Programming Language", 4th ed. as
>well as recent talks he has given on the nature of C++:

I tend to find appeals to authority less of interest than the
experience gained over 30+ years developing very large C++ software projects.

>The strength of C++ is that it provides the ability to create abstractions
>while still yielding efficient code execution. In many cases the
>abstractions are literally free when compared to the low-level detail
>oriented code you would need to write in C to achieve the same end result.

I don't discount at all the advantages provided by object
abstraction - I use them heavily; particularly pure abstract classes
as 'interfaces' in the java sense.

I don't believe in the fanatical pursuit of language purity at the
expense of functionality, readability or performance.

[Richard]

[Please do not mail me a copy of your followup]

sl...@pacbell.net spake the secret code

<zSGAx.1903$Md3....@fx01.iad> thusly:

>snprintf vs. "<<" and ">>" should be sufficient to show
>how the readability (and maintainability) of C++ code is
>improved by selective use of C run-time library functions.

I disagree because of the large number of errors that I have seen
w.r.t. use of printf style functions over the years. You are, of
course, free to do whatever you want. But I will continue to counsel
against it based on many years of experience on many teams where
entire classes of bugs simply disappeared from our code because we
stopped using printf style code.

I would make an exception where the performance is important and measured
to be slower with stream insertion operators and this was the dominant
factor in the performance analysis. However, this is a case where we
are intentionally using more error-prone means to achieve a given end
because other factors are an overriding concern. Even here, there
are wrappers C++ around printf that can alleviate some, but not all,
of it's problems and these would be preferred over direct usage of printf.

>You, like Stefan, seem to think that only "pure" C++ is
>useful or valid, which is far from the actual truth.

I have no idea what "pure" C++ is according to you. I haven't said
any such thing in any of my posts.

I'm saying that C programming habits routinely introduce problems that
are easily avoided by using C++ programming habits instead. Among
other things, that means preferring the C++ standard library over the
C standard library. Please read that sentence carefully, because I
don't want it to be re-interpreted into another straw-man argument
about some absolutist style "demand".

>>By all means, if you want to write C, then please head on over to
>>comp.lang.c, contribute to the unix kernel, or whatever.
>
> Believe that I _have_ contributed to both the unix kernels and
>the linux kernels (and even the oracle RDBM core).

Great for you. Please do continue to write C and contribute to C
based projects as you see fit. However, I will continue to counsel
against C-style programming when programming in C++. While C++ may be
mostly compatible with C, my advice has been and will continue to be
that you should stop thinking C when writing C++ and think in C++
terms instead.

>>However,
>>please stop teaching people that writing good C++ means using C-isms.
>
> However, you have no right to make such a silly demand.

Where am I making a "demand"? That is you creating a straw man
argument by making up things I haven't said. I will continue to
advise and request that people stop using C style habits when
programming in C++.

>There
>are many cases where using C run-time library functions and C
>idioms in C++ code is perfectly valid. It's perfectly legal use
>of the C++ language as well.

Another straw-man argument. Nowhere have I said it was invalid or
illegal to use C style programming habits.

I have said, and will continue to say, that programming with C-isms
(e.g. using C style programming habits) is an error-prone, unnecessarily
low-level (e.g. devoid of higher levels of abstraction) and therefore
much more verbose than the C++ equivalent mechanisms.

>I tend to find appeals to authority less of interest than the
>experience gained over 30+ years developing very large C++ software projects.

My point was less an appeal to authority as simply pointing out that
I am not sone "lone usenet net.crank" in advocating this point of
view. It is, in fact, quite commonly advocated among the leading C++
practitioners in the field, which includes the designer of the language.
If you want to interpret that as an appeal to authority, then fine,
by all means ignore my advice as well as the similar advice of people
like Herb Sutter, Bjarne Stroustrup, James McNellis, Kate Gregory,
all the wonderful speakers at places like CppCon, C++ Now! and so-on.
I am not saying that automatically all those people agree with me 100%
on everything, but I am saying that the advice I've been giving here in
this newsgroup is consistent with the same advice I hear from them when
I listen to their talks on youtube or when I have had the pleasure of
talking to them in person.

I state my recommendations and advice based on real-world experience in
many teams at different companies over a long period of time. I have led
instruction in the C++ standard library to make teams aware of how C++
and the standard library provides tools for common programming problems
(such as string manipulation; which is how this whole tangent got started)
that are less error-prone than their C counterparts. As a result,
entire classes of problems simply disappeared from our code because we
stop writing code in the C style and embraced C++ fully.

>I don't believe in the fanatical pursuit of language purity at the
>expense of functionality, readability or performance.

The "fanatical pursuit of language purity" you speak of seems to be
embodied by the straw man arguments cited earlier in this post. I
don't even know what "language purity" means; you are the one who is
introducing these terms, not me.

I don't even understand how one can argue that C style programming
embraces more functionality than C++ style programming since you've
already admitted C++'s ability to be (largely) backwards compatible
with C.

Readability is a subjective phenomenon and I don't know any way to
objectively measure it in a manner that all programmers can agree with
the measurement. I am more than willing to describe examples of C style
programming that I think impair readability. It almost always is a
result of insufficient use of abstraction that impairs the ability to
express directly the intention of the programmer. In my day-to-day
programming within a team my suggestions to improve readability almost
always revolve around eliminating duplication, simplifying syntax and
introducing abstractions with intention revealing names in order to
improve readability.

As for performance there are specific examples where C++ code outperforms
C standard library functions and programming habits. There are also
cases where the performance of a C style approach, such as printf,
exceeds C++. However, the exception shouldn't become the rule. By that I
mean that simply because printf can outperform stream insertion for some
particular piece of code, doesn't mean that one should systematically
reject all things C++ for all things C. Nor does it mean "stream
insertion operators are bad" because most of the time the performance
difference isn't significant and there are advantages of stream
insertion operators over printf.

If you want to use printf and other C style programming habits
routinely, then my recommendation is to not program in C++ and instead
program in C. Then you can advocate C style programming practices to
the C community instead of advocating C style programming to the C++
community. I will continue to advocate C++ style programming habits
and the avoidance of C style programming habits to the C++ community.

[Jorgen Grahn]

On Mon, 2015-08-17, Scott Lurndal wrote:
> legaliz...@mail.xmission.com (Richard) writes:
>>[Please do not mail me a copy of your followup]
>>
>>sl...@pacbell.net spake the secret code
>><yHlAx.65234$MQ3....@fx13.iad> thusly:
>>
>>>Using std::string instead of a char buffer + strcat would avoid this.
>>
>>Another perfect example of why you should stop using C-isms in C++.
>
> And of course, counter-examples abound. snprintf could have been
> used in place of strcat (and would have avoided the buffer overflow).
>
> Even proper use of strncat would have sufficed.

"Proper" often means detecting that truncation has happened, and
somehow correcting it. That can easily become a lot of code ...

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .