[m0dem]

You can hardcode data and still not have string data show up with a program like strings if you encode it as integers or encrypt the string first, and at runtime decrypt them. Strings will not execute your binary to determine what the strings actually are, so it would require some debugging at that point.

Your first paragraph is good ... useful constructive criticism. I'll give you that.

Otherwise, go troll someone else...

Although this is actually very horribly written code to begin with...

Would you care to elaborate on how this is "horribly written code." And to whose standard? Yours?
Frankly, I don't care if someone doesn't "like" my code. If it works beautifully, it is beautiful.
(not that my program does work perfectly)

In addition to the size of HOST being 20, what's the point, even the longest IPv4 address can be 15 characters in length, so the buffer will never have to be any larger than 16.

^ COMPLETELY pointless

---

Show me your skillz...

[Penis]

You can hardcode data and still not have string data show up with a program like strings if you encode it as integers or encrypt the string first, and at runtime decrypt them. Strings will not execute your binary to determine what the strings actually are, so it would require some debugging at that point.

Although this is actually very horribly written code to begin with...

---

Exactly, and furthermore stdout is typically buffered, whereas stderr isn't for obvious reasons.

---

In addition to the size of HOST being 20, what's the point, even the longest IPv4 address can be 15 characters in length, so the buffer will never have to be any larger than 16.

Or you can have an array of characters and combine them at runtime, that doesn't show up in strings, also OP you should implement Ipv6 support

[dotcppfile]

Your first paragraph is good ... useful constructive criticism. I'll give you that.

Otherwise, go troll someone else...


Would you care to elaborate on how this is "horribly written code." And to whose standard? Yours?
Frankly, I don't care if someone doesn't "like" my code. If it works beautifully, it is beautiful.
(not that my program does work perfectly)


^ COMPLETELY pointless

---

Show me your skillz...

It is badly written, I said why in my last post and there's a lot more to add to it... Here's a summary of it and bit more:
1- popen is bad for shells since it doesnt get stderr
2- the cd command isnt implemented
3- your shell cant handle interactive programs like ftp for example, you'll lose it if you do so.
4- your shell cant handle processes that takes a long time, for example an unlimited ping, your shell will simply wait for it forever which means that we lost the shell.
5- your whole shell cant handle any exception at all...

So for the shell? Its bad and not safe, I wouldnt use it nor will anyone that wants a proper shell or cares about his shells in the first place.

As for your HOST variable than its a dumb waste of memory even if its about a byte or two...

As for your "if it works beautifully then its beautiful", you cant be serious mate lol its not about making it work, you can always make it work in a way or another but what makes it better than the others is how it works, how it handles everything, CPU usage, memory management so whatever...

Dont take this offensively, its all true and you know it.

[m0dem]

It is badly written, I said why in my last post and there's a lot more to add to it... Here's a summary of it and bit more:
1- popen is bad for shells since it doesnt get stderr
2- the cd command isnt implemented
3- your shell cant handle interactive programs like ftp for example, you'll lose it if you do so.
4- your shell cant handle processes that takes a long time, for example an unlimited ping, your shell will simply wait for it forever which means that we lost the shell.
5- your whole shell cant handle any exception at all...

So for the shell? Its bad and not safe, I wouldnt use it nor will anyone that wants a proper shell or cares about his shells in the first place.

As for your HOST variable than its a dumb waste of memory even if its about a byte or two...

As for your "if it works beautifully then its beautiful", you cant be serious mate lol its not about making it work, you can always make it work in a way or another but what makes it better than the others is how it works, how it handles everything, CPU usage, memory management so whatever...

Dont take this offensively, its all true and you know it.

Note the title: "Simple Reverse Shell in C"
Its just for beginners to understand what is put into something like this.
If I added a bunch more stuff, a beginner would not be able to clearly understand it.
People have to start somewhere.

Thanks for the feedback and stuff.
I'll look for better ways of doing things.

[0xDEAD10CC]

Your first paragraph is good ... useful constructive criticism. I'll give you that.

Otherwise, go troll someone else...


Would you care to elaborate on how this is "horribly written code." And to whose standard? Yours?
Frankly, I don't care if someone doesn't "like" my code. If it works beautifully, it is beautiful.
(not that my program does work perfectly)


^ COMPLETELY pointless

---

Show me your skillz...

I'm not trolling. This is just you not being able to accept valid points.

Horribly written code:
1. memset() comes from the <string.h> header where it is properly prototyped, which you haven't included in your program. This is also the same header where strlen() comes from.
2. You define pointer variables that you don't assign to NULL for safety, but you also separate declaration and definition for no reason in a few places even if this is ANSI C compliant code (but it isn't because you use VLA's).

Code:

FILE *fp; fp = popen(cmd, "r");

3. 'const char HOST[20]' could have been 'const char HOST[]'
4. Even if socket creation fails, you print an error and continue with the program to printf("Socket created.\n") which is ridiculous.
5. In some cases where function call errors will set a value that WSAGetLastError() can provide, you don't call it, but you return from main() still.
6. You memset() the command buffer, even though you set a null terminator which is redundancy
7. if recv() fails, you puts("recv failed") and continue on with the regular program flow as if nothing bad happened
8. You never call the respective pclose() function for any of your popen() calls
9. You never call WSACleanup() in any of the error branches after WSAStartup()

- Minimal error checking if any in some places
- No functions
* Bad code formatting with indentation

I could continue on too...

And why is my suggestion about the size of the HOST buffer completely pointless? IMO it's more pointless that you don't know or don't allocate for the space that you need. Additionally 20 is not a great number for anything really.

If you took the time to look around the forum you'd see my contributions btw. Clearly you're too lazy for that kind of task.

As for your mention about "Simple" being in the title. "Simple" is not a synonym for badly written code. If anything, it should make it more prone to well-written code, which isn't the case here, so that is a poor excuse. This code would also never be optimized for any code caches because the numbers you've chosen look like they were picked from a rabbits ass and not anything that required meaningful thought. HOST as 20 is proof enough for that.

[m0dem]

I need to apologize for coming across in a bad way. I really want to learn, but sometimes my temper can get in the way.

Thank you for the list of reasons why my code was bad. I really do appreciate it.
I have fixed up my code in mostly all of the points you listed. Some of your solutions had never entered my mind.

If you took the time to look around the forum you'd see my contributions btw. Clearly you're too lazy for that kind of task.

In fact, before posting previously, I was reviewing some of your posts.

---

I have searched far and wide on how to open a pipe to cmd.exe and then read and write to the same process over and over again.
I know how to read from one process and write to one process, but I can not figure out how to read and write to the same process.
Maybe someone has suggestions? Thanks!

[dotcppfile]

I have searched far and wide on how to open a pipe to cmd.exe and then read and write to the same process over and over again.
I know how to read from one process and write to one process, but I can not figure out how to read and write to the same process.
Maybe someone has suggestions? Thanks!

Pipes: https://msdn.microsoft.com/en-us/library...85%29.aspx

[0xDEAD10CC]

OK, that looks a bit better. The whole idea of functions is to reduce the need to write repetative code and code that has to check and parse things when you can delegate that to something else. Instead, you should try writing functions that will allow you to achieve an implementation like this:

Code:

#include "sockets.h" #include <stdio.h> #include <stdlib.h> int main(void) { int err; if (!socket_initialize(&err)) { fprintf(stderr, "socket_initialize() failed: %d\n", err); exit(1); } /* ... */ socket_cleanup(); exit(0); }

The else in this case is redundant though:

Code:

// create the Winsock socket if ((s = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET) { printf("Could not create socket: %d" , WSAGetLastError()); return 1; } else { printf("Socket created.\n"); }

Take a close look With the return there, there's no need to worry about any other code being ran outside of the if statement structure or within any of the internal code branches if the first condition is met. Thus, it's the same as this:

Code:

// create the Winsock socket if ((s = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET) { printf("Could not create socket: %d" , WSAGetLastError()); return 1; } printf("Socket created.\n");

[m0dem]

OK, that looks a bit better. The whole idea of functions is to reduce the need to write repetative code and code that has to check and parse things when you can delegate that to something else. Instead, you should try writing functions that will allow you to achieve an implementation like this:

Code:

#include "sockets.h" #include <stdio.h> #include <stdlib.h> int main(void) { int err; if (!socket_initialize(&err)) { fprintf(stderr, "socket_initialize() failed: %d\n", err); exit(1); } /* ... */ socket_cleanup(); exit(0); }

The else in this case is redundant though:

Code:

// create the Winsock socket if ((s = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET) { printf("Could not create socket: %d" , WSAGetLastError()); return 1; } else { printf("Socket created.\n"); }

Take a close look With the return there, there's no need to worry about any other code being ran outside of the if statement structure or within any of the internal code branches if the first condition is met. Thus, it's the same as this:

Code:

// create the Winsock socket if ((s = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET) { printf("Could not create socket: %d" , WSAGetLastError()); return 1; } printf("Socket created.\n");

Maybe I'm not understanding you correctly, but I don't need to initialize a socket or clean up a socket multiple times. So is it really beneficial to enclose those actions inside a function?

I fixed that useless else statement. Thanks for catching that.

[0xDEAD10CC]

Maybe I'm not understanding you correctly, but I don't need to initialize a socket or clean up a socket multiple times. So is it really beneficial to enclose those actions inside a function?

I fixed that useless else statement. Thanks for catching that.

It was a principle that I was trying to show, not that you should have that exact same code, however that doesn't mean that you can't have a function which will set up your socket for you so that it's ready for use; socket options set, bindings, etc...

If you have descriptive function calls, the structure and flow of your program is much easier to read. Ex: *note - just pseudocode*

Code:

init(flag); while (flag) { for (int i = ....) calculate(&x, &y, &z); x += somevalue; y -= x; z = function(x, y); if (condition) flag = false; }

Or:

Code:

init(flag); while (flag) { if (!function(&x, &y, &z) flag = false; }

^^ This makes it much easier to see the flow of your program which isn't complicated and cluttered with the calculations that you may not have to see unless you have to troubleshoot your algorithm. Keep in mind, I have no idea what kind of code would look like this or if this structure would even be useful. I'm just showing how functions can allow your code to be more readable in lots of cases, and unless you're concerned with micro-optimizing your code and reducing a single function call (which can still be optimized in other ways, even if it isn't inlined), you should care more about the readability of your code.

If I have to add code to that while loop too. When writing larger projects, it's nice to know that I'm not going to break the good code that I've well tested, by modifying the body of my loop, whereas in the first example, I have to read over the code and examine it to understand what I shouldn't touch as it would break code that is working fine.