0

I am experimenting with using system commands in C++, and I am trying to make a pinger. Below is my script:

#include <stdlib.h>
#include <iostream>
#include <stdio.h>
using namespace std;

int main()
{
char ip;
char *cmd;
cout << "Input IP: ";
cin >> ip;
sprintf(cmd, "ping %s", ip);
system(cmd);
return 0;
}

The code compiles and runs fine until you enter the IP you want to ping, at which point it gives me this:

Input IP: 8.8.8.8
Segmentation fault (core dumped)

I suspect it has something to do with sprintf, but I'm not sure since I'm a beginner when it comes to coding in C++

How do I fix this error?

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
neliah
  • 47
  • 5
  • where do you want to store the input? Where does `cmd` point to? Why not use `std::string`? – 463035818_is_not_an_ai Dec 24 '21 at 00:51
  • The input is stored in the variable ip. cmd doesn't print anywhere, it gets ran in a windows command line. And what would be the benefits of using string instead of chars? Please excuse my naivety, I'm very now to C++ – neliah Dec 24 '21 at 00:53
  • 2
    You seem to be in need of a good C++ book: https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – 463035818_is_not_an_ai Dec 24 '21 at 00:53
  • 2
    a `char` is a single character. A `char*` is just a pointer. If you want a string then you can use `std::string` – 463035818_is_not_an_ai Dec 24 '21 at 00:54
  • 1
    btw better don't call `system` with user input – 463035818_is_not_an_ai Dec 24 '21 at 00:55
  • I will try using string instead of char, one moment. Thank you for the book, I'll be sure to give it a read. – neliah Dec 24 '21 at 00:55
  • 2
    think about what happens when someones ip is `100.0.0.0 ; rm *` – 463035818_is_not_an_ai Dec 24 '21 at 00:56
  • @463035818_is_not_a_number it's a bit like the log4j leak for the poor :-D – πάντα ῥεῖ Dec 24 '21 at 00:57
  • I don't know, what happens when someones ip is "100.0.0.0 ; rm *" ? – neliah Dec 24 '21 at 00:59
  • 1
    on linux your code would delete all files in the working directory. In windows its called `del` I think – 463035818_is_not_an_ai Dec 24 '21 at 01:01
  • Considering that `sprintf()` (and the like) are often a source of attack vectors, I'd strongly recommend using `snprintf()`. That would sort of bring home the need for already allocated memory being needed. As this is C++, I would personally use `std::ostringstream` and avoid many of the problem with the `printf()` family of functions (and I'm sure people will point that I would get a host of other problems in return - however, overall the type safety and added run-time safety are worth it, in my opinion). – Dietmar Kühl Dec 24 '21 at 01:20
  • Also, I see input using `cin >> ip;` without a check whether this input was successful. You should **always** test whether input was successful after the input function, e.g., using `if (!(cin >> ip)) { /* deal with failed input */ }`. ... and, of course, before using the result of input you may want to make sure it contains the expected content - [Little Bobby Tables](https://xkcd.com/327/) comes to mind... – Dietmar Kühl Dec 24 '21 at 01:23

1 Answers1

0

I strongly suggest to not mix C and C++ when not needed. That means, use the C++ versions of the C headers and only use the C headers when you need to. In C++ there is std::string which can be concatenated via +.

In your code ip is a single character and cmd is just a pointer. You fail to make it point to some allocated memory. Hence the runtime error when trying to write to the memory pointed to by cmd.

#include <string>
#include <iostream>
#include <cstdlib>
    
int main() {
    std::string ip;
    std::cout << "Input IP: ";
    std::cin >> ip;
    std::string cmd = std::string{"ping "} + ip;
    std::system(cmd.c_str());
}

Note that calling system with user input is a security risk. It lets the user execute arbitrary commands. I strongly suggest to at least check that ip is a valid ip and not something else. See here: How do you validate that a string is a valid IPv4 address in C++?

Pepijn Kramer
  • 9,356
  • 2
  • 8
  • 19
463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185