0

This is for a homework assignment. I've been given a c++ file that is supposed to demonstrate a buffer overflow error and I need to correct the error. Unfortunately, I can't reproduce the error to begin with. Two local variables are declared right next to each other, presumably with the idea that when the character array is given a value that is too large the overflow will go into the next variable and that will now display incorrectly.

Here is the code that I was given (with some comments removed):

#include <iomanip>
#include <iostream>

int main()
{
  std::cout << "Buffer Overflow Example" << std::endl;

  const std::string account_number = "CharlieBrown42";
  char user_input[20];
  std::cout << "Enter a value: ";
  std::cin >> user_input;

  std::cout << "You entered: " << user_input << std::endl;
  std::cout << "Account Number = " << account_number << std::endl;
}

However, when I type more than 20 characters into the prompt it still returns the full string that I've entered and then returns the correct value for the account_number. My understanding is that the extra characters that I enter should bleed into the account_number.

Under the project properties I've already tried turning off Basic Runtime Checks (under C/C++->Code Generation) and I've turned off Randomized Base Address (under Linker->Advanced). Is there some other setting that I need to change to be able to produce a more predictable buffer overflow?

Tom H
  • 46,766
  • 14
  • 87
  • 128
  • 2
    UB is UB. Plus the actual string data for account_number is on the heap – pm100 Mar 09 '23 at 19:18
  • _Unfortunately, I can't reproduce the error to begin with_ I don't think you need to. Just fix the code (hint: don't use fixed-size buffers or C-style arrays). – Paul Sanders Mar 09 '23 at 19:20
  • 1
    14 characters should fall in SSO on every compiler, so it wouldn't be on the heap, but it's likely that compiler optimised away `account_number` entirely, simply substituting the string where it's needed. It would be the same as [this](https://stackoverflow.com/questions/3593687/two-different-values-at-the-same-memory-address) [repeating](https://stackoverflow.com/questions/2508605/modifying-a-const-through-a-non-const-pointer) [question](https://stackoverflow.com/questions/22656734/how-is-a-variable-at-the-same-address-producing-2-different-values?noredirect=1&lq=1). – Yksisarvinen Mar 09 '23 at 19:21
  • 5
    since c++20 your code will work perfectly, the stream won't overflow the buffer https://en.cppreference.com/w/cpp/io/basic_istream/operator_gtgt2 https://godbolt.org/z/7WT5qYKWc – Alan Birtles Mar 09 '23 at 19:23
  • 1
    "I'm trying to make undefined behavior behave in a defined way". Well, good luck with trying to do this. – PaulMcKenzie Mar 09 '23 at 19:40
  • 1
    *Two local variables are declared right next to each other* -- The program that you write is only a description of what you want done. See the [as-if](https://en.cppreference.com/w/cpp/language/as_if) rule. It is up to the compiler as to where and how those variables are stored. – PaulMcKenzie Mar 09 '23 at 19:43
  • @AlanBirtles make an answer out of your comment. But you need to provide an example of buffer overflow UB too. That means you may need to call C functions. This could make a good example on capacity of modern C++ to overcome unsafeties of C. I suggest the OP writes an article on this for his class. – Red.Wave Mar 09 '23 at 20:38
  • @Red.Wave I don't think my comment is the answer to the question (hence it being a comment), they're probably not using c++20 as they say they get a string longer than 20 characters printed out, the c++20 version would truncate to 19 characters – Alan Birtles Mar 09 '23 at 22:29
  • @AlanBirtles this is harming C++. Regarding those junk heads in NSH, this is not the fault of lazyass instructors. Everybody should conform to the latest standards. – Red.Wave Mar 10 '23 at 16:38

2 Answers2

2

One way to achieve this would be to place the buffer inside a struct and place a "magic number" after it:

struct ProtectedBuffer
{
    char buffer[20];
    unsigned magicNumber = 1234;

    bool overflow() {magicNumber != 1234;}
};

This way, any overflow of the buffer will bleed into the fixed magic number which can be detected.

nielsen
  • 5,641
  • 10
  • 27
  • Note that alignment may cause the compiler to add padding between the fields. So, any overflow would have to be sufficient enough to overcome that padding in order to corrupt the magic number. – Remy Lebeau Mar 09 '23 at 20:24
0

Compilers can do all sorts of optimizations, and there is also no guarantee that two local variables are layed out in a deterministic order on the stack. Furthermore, std::string allocates the memory for its contents on the heap, unless small string optimization is used. The latter depends on the operating system and the CPU architecture (in particular, whether it is a 32 bit or 64 bit CPU).

As Alan Birtles mentions, since C++20 the operator<< overload will see that you are reading into a fixed-size array and wil not cause it to write out of bounds.

A compiler could also decide that nothing can legally overwrite the const string account_number, so it could just not generate a local variable on the stack for it, but instead store the string in a const data section in the binary.

G. Sliepen
  • 7,637
  • 1
  • 15
  • 31