I suggest avoiding the use of magic numbers can often make the code less likely to have buffer over runs.
Tim S.
I suggest avoiding the use of magic numbers can often make the code less likely to have buffer over runs.
Tim S.
"...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson
It appears that I need to use strncpy instead of strcpy in order to put an upper bound on the characters that can be copied..?
No, you don't. The reason why we would use strcpy or strncpy (and other related alternatives) is that we have an original string that is not to be modified, and yet we want to modify it, so we copy the string to a buffer and modify the buffer instead. In this case, the original string argv[1] isn't modified, and even if it were modified, there's no issue modifying it, hence you don't need to copy in the first place.Originally Posted by TheCMan
Another thing: you need to check that the user did in fact supply a command line argument, otherwise argv[1] would likely be a null pointer instead of a string.
Look up a C++ Reference and learn How To Ask Questions The Smart WayOriginally Posted by Bjarne Stroustrup (2000-10-14)
Yes but the statement strcpy(buffer, argv[1]) copies the characters in argv[1] to buffer without any sort of bounds checking. So, if there are more than 100 characters, it will start overwriting the stack after the end of the space allocated for buffer. And yes, the command line to turn off protections and disable ASLR so it can be exploited:
Then I can simply inject shell code onto the stack using a Perl file.Code:gcc –o vuln –fno-stack-protector –z execstack vulnerable.c chmod 4755 vuln sudo sysctl –w kernel.randomize_va_space=0
Okay. So I tell you how to avoid the vulnerability (don't copy in the first place), and your response is to tell me that the use of strcpy is a vulnerability in this case when I told you that in post #6. *shrugs*
Look up a C++ Reference and learn How To Ask Questions The Smart WayOriginally Posted by Bjarne Stroustrup (2000-10-14)
strcpy is the culprit yet I still do not know how to replace it to make not vulnerable after post #20. *shrugs*
I already told you how to fix it: don't copy in the first place! Directly use argv[1]. You cannot have a buffer overflow vulnerability with strcpy when you never call strcpy at all.
You could fix the vulnerability by changing to strncpy, that's true. But it comes with two disadvantages: you would be making an unnecessary assumption concerning the maximum length of user input, and you would need to get the strncpy call correct (not hard, but mistakes can and do happen).
Last edited by laserlight; 05-18-2019 at 08:18 AM.
Look up a C++ Reference and learn How To Ask Questions The Smart WayOriginally Posted by Bjarne Stroustrup (2000-10-14)
I guess I'm not understanding what you mean when you say use argv[1]. Use it where?
"...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson
So can anyone help me get the code to not be vulnerable?
Use it instead of buffer.Originally Posted by TheCMan
No, I didn't; I just didn't bother mentioning it in post #6. Did you read my post #8?Originally Posted by stahta01
Aren't we helping you? What is your current code?Originally Posted by TheCMan
Also, I note that you didn't state what the code was actually supposed to do, so we can only guess based on the code itself. This leads to the curious situation where it looks like you want to reinterpret the bytes to print an unsigned int, yet you use a long* and have a comment about "four-byte word". It makes me wonder if you should just stick to unsigned int or unsigned long with sizeof.
Last edited by laserlight; 05-18-2019 at 02:22 PM.
Look up a C++ Reference and learn How To Ask Questions The Smart WayOriginally Posted by Bjarne Stroustrup (2000-10-14)
The current code is posted at the beginning. And I said what the code was supposed to do- to not be vulnerable to stack buffer overflow attack. Does anyone want to post a solution, using the current code I have so that I can test it?
Which means that you have done nothing to improve it despite suggestions?Originally Posted by TheCMan
That's not what the code is supposed to do in the sense of functional requirements. That's a non-functional requirement, as important as it may be. If it were a functional requirement, then we could change the code to:Originally Posted by TheCMan
and it would be correct... which I think you would agree is absurd.Code:int main(void) { return 0; }
Nope, not yet. Improve your code based on suggestions and post your updated code first. I actually wrote it for fun yesterday, but I decided not to post it partly because I noticed the long vs unsigned int situation.Originally Posted by TheCMan
Last edited by laserlight; 05-18-2019 at 02:50 PM.
Look up a C++ Reference and learn How To Ask Questions The Smart WayOriginally Posted by Bjarne Stroustrup (2000-10-14)
No, it means your suggestions haven't been helpful. I don't need an explanation on what is and isn't a functional requirement. The question isn't about the function of the code, it's about the vulnerability of the code, which I guess you are yet to understand. The question is how to prevent the method from being vulnerable to stack buffer overflow attacks. I don't need suggestions, I need a solution and to understand why the solution is not vulnerable.
No, I understand this perfectly fine. Since you consider yourself too stupid to make use of suggestions, and I'm getting tired of arguing, here's your solution:Originally Posted by TheCMan
Code:#include <stdio.h> #include <stdlib.h> #include <string.h> int main(int argc, char **argv) { if (argc <= 1) { fprintf(stderr, "Error: please provide one command line argument.\n"); return EXIT_FAILURE; } size_t len = strlen(argv[1]); if (len % sizeof(unsigned int) != 0) { fprintf(stderr, "Error: the input must be a multiple of %zu characters.\n", sizeof(unsigned int)); return EXIT_FAILURE; } unsigned int *addr_ptr = (unsigned int*)argv[1]; for (size_t i = 0; i < len / sizeof(unsigned int); ++i) { printf("%02zu:%p:%08x\n", i, (void*)addr_ptr, *addr_ptr); addr_ptr++; } return 0; }
Last edited by laserlight; 05-18-2019 at 03:26 PM.
Look up a C++ Reference and learn How To Ask Questions The Smart WayOriginally Posted by Bjarne Stroustrup (2000-10-14)