Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buffer overflow is not a buffer overflow #4

Closed
Nordgaren opened this issue Feb 18, 2024 · 49 comments
Closed

buffer overflow is not a buffer overflow #4

Nordgaren opened this issue Feb 18, 2024 · 49 comments
Assignees

Comments

@Nordgaren
Copy link

Print out the addresses of each box.

There is no guarantee that Box will allocate the buffer in the same position on the heap.

Your code as it is now will have the old buffers address as 0x1, and not the same address as the new buffer.

If I println!("{:?}", original_buffer.as_ptr();) in get_dropped_buffer it prints what you expect it to print for the address. I assume this as something to do with rust optimization even though you have everything in black_box. The buffer address still changes around, though. I will have to take a look at the actual asm to see what is causing this.

Can you tell me what version of the compiler you guys did this on?

Thanks, and cool stuff!
Nordgaren

@Speykious Speykious assigned Speykious and Creative0708 and unassigned Speykious Feb 18, 2024
@Nordgaren
Copy link
Author

Actually, I know what is happening. the Box::new() is another function, and so is the constructor for the slice, probably, so that stack frame is clobbering the buffer_ref variable, I think.

I will look at asm to confirm. Also thanks for the quick assignment.

@Speykious
Copy link
Owner

You're welcome!

I usually do things on the latest available stable version, I don't know about @Creative0708 though.

@Speykious
Copy link
Owner

Ah woops, @Bright-Shard made the buffer overflow, my bad :v

@Bright-Shard
Copy link
Collaborator

Bright-Shard commented Feb 18, 2024

There is no guarantee that Box will allocate the buffer in the same position on the heap.
Your code as it is now will have the old buffers address as 0x1, and not the same address as the new buffer.

The buffers don't need to be in the same place. What needs to be the same is the location on the stack of the two Boxes we create. What this allows us to do is swap a pointer to one buffer with a pointer to another buffer, since we drop the first box and then put a new box in its place, and maintain a reference to that same location.

On its own this is useless, except that the two buffers have different sizes, and Rust doesn't realise the size has changed. I'm not sure if it's a fat pointer thing or a type system thing - all that matters is that when we loop over every item in the buffer, Rust loops ten times, since the size of the original buffer is 10. Under the hood, we've swapped the pointer to point to a buffer of size 5, meaning we end up writing 10 items to a buffer with 5 items - a buffer overflow.

Now that you've pointed this out, though, I think the printing logic is flawed; we should find a way to print 10 items from the second buffer's address, to prove that we've written 10 items to it. Currently I can't think of a way to do this in "safe" code though.

@Speykious
Copy link
Owner

Speykious commented Feb 18, 2024

If it's just for proof printing inside the CLI, maybe some unsafe printing is ok? Just like we do in tests.

@Nordgaren
Copy link
Author

Nordgaren commented Feb 18, 2024

There is no guarantee that Box will allocate the buffer in the same position on the heap.
Your code as it is now will have the old buffers address as 0x1, and not the same address as the new buffer.

The buffers don't need to be in the same place. What needs to be the same is the location on the stack of the two Boxes we create. What this allows us to do is swap a pointer to one buffer with a pointer to another buffer, since we drop the first box and then put a new box in its place, and maintain a reference to that same location.

On its own this is useless, except that the two buffers have different sizes, and Rust doesn't realise the size has changed. I'm not sure if it's a fat pointer thing or a type system thing - all that matters is that when we loop over every item in the buffer, Rust loops ten times, since the size of the original buffer is 10. Under the hood, we've swapped the pointer to point to a buffer of size 5, meaning we end up writing 10 items to a buffer with 5 items - a buffer overflow.

Now that you've pointed this out, though, I think the printing logic is flawed; we should find a way to print 10 items from the second buffer's address, to prove that we've written 10 items to it. Currently I can't think of a way to do this in "safe" code though.

I don't see how Rust thinks the size changed. You are still writing to a &mut Box<[u8; 10]> with your first reference, which is what you are iterating over. So these two things are totally unrelated. you aren't overflowing a 5 byte buffer. You are just writing out of bounds into random memory.

@Nordgaren
Copy link
Author

If you remove the println from your code, the language server will tell you that you are not using the five byte buffer at all, so how could you be overflowing it?

@Nordgaren
Copy link
Author

image
The iterator that you get is even calling with the correct size of the buffer you are writing to.

@Bright-Shard
Copy link
Collaborator

Bright-Shard commented Feb 18, 2024

Look at the printout:
image

As you say, the only time we use the five byte buffer is to print from it. So then how did the values change?

Here's what happens in the code:

  1. We create a box to a 10-byte buffer.
  2. We use lifetime expansion to create a &'static mut reference to that box.
  3. The box gets dropped at the end of the function. Note that the reference we created is 'static, so it's still valid.
  4. We create a new box that now points a 5-byte buffer. It's a little hacky but the box gets placed in the exact same place on the stack. This means the &'static mut reference we created before is now pointing to our 5-byte buffer box.
  5. We fill the buffer with 3s using our reference. Because Rust thinks the reference is pointing to a 10-byte buffer, it writes ten 3s. However, because of what happened in 4, we've written those ten 3s to the 5-byte buffer, not the 10-byte buffer (the 10-byte buffer doesn't even exist at this point, it's not existed since step 3).

In short, all of this code would be correct if it was still using that box to a 10-byte buffer. But we dropped the box to that 10-byte buffer. We swapped it with a box pointing to a 5-byte buffer. So it's now overflowing.

@Nordgaren
Copy link
Author

"buffer you are writing to"

I put quotations, because it seems that the pointer is changing after some function calls. I can't quite figure out why yet. I do thing this is at least use after free. A colleague of mine pointed out that the heap caching won't count for the smaller size. I think possible there are two allocations here. One for the 10 byte buffer and one for the 5 byte buffer.

@Bright-Shard
Copy link
Collaborator

Yes - the original bug all of our "features" exploit is a use-after-free. Here we exploit that use-after-free to change the pointer so it points to a 5-byte buffer. Rust doesn't realise the size has changed and thus still writes 10 times (the size of the old buffer).

@Nordgaren
Copy link
Author

Look at the printout: image

As you say, the only time we use the five byte buffer is to print from it. So then how did the values change?

Here's what happens in the code:

1. We create a box to a 10-byte buffer.

2. We use lifetime expansion to create a `&'static mut` reference to that box.

3. The box gets dropped at the end of the function. Note that the reference we created is `'static`, so it's still valid.

4. We create a new box that now points a 5-byte buffer. It's a little hacky but the box gets placed in the _exact same place_ on the stack. This means the `&'static mut` reference we created before is now pointing to our 5-byte buffer box.

5. We fill the buffer with `3`s using our reference. Because Rust thinks the reference is pointing to a 10-byte buffer, it writes ten 3s. However, because of what happened in 4, we've written those ten 3s to the 5-byte buffer, _not_ the 10-byte buffer (the 10-byte buffer doesn't even exist at this point, it's not existed since step 3).

image
Here is my print out. It has an access violation on my end.

It's not the same. I still don't know if I would consider it a buffer overflow, because, again, box::new is not guaranteed to give you the same address. Even on your machine with your compiler, a multi-threaded program might allocate something in the meantime.

@Nordgaren
Copy link
Author

This is def Use After Free, with the potential to be a buffer overflow, depending on compiler and program.

@Bright-Shard
Copy link
Collaborator

What I'm trying to explain is that we aren't trying to allocate twice to the same address. We're swapping the boxes themselves - ie, the pointers, not the data they point to. Please reread my step-by-step explanation.

Probably the reason yours segfaults there is this exploit relies on both boxes getting put in the exact same place on the stack. This works for me on linux, but I guess it doesn't on windows. It's hard to guarantee that they do get put in the same stack location without unsafe code.

@Nordgaren
Copy link
Author

But you aren't. You aren't telling Rust that you are writing 10 bytes to a 5 byte buffer. That would be overflowing the buffer. You are writing 10 bytes to memory that has already been freed. You aren't even using the 5 byte buffer. You are POTENTIALLY using it.

But it doesn't have the same behavior in all cases.

@Nordgaren
Copy link
Author

Nordgaren commented Feb 18, 2024

image

this is if I print out the address in the get_dropped_buffer method.

How is this a buffer overflow at all? Not using the same variables, even.

@Creative0708
Copy link
Collaborator

I think there's a misunderstanding here. The reference to the buffer is a &mut Box<[u8; 10]>, not a Box<[u8; 10]>. The code isn't relying on the two buffers being allocated on the same spot on the heap; it's relying on the two boxes to the buffer being placed on the same spot on the stack.

@Speykious
Copy link
Owner

Aha, it's a double reference indeed! I was starting to have doubts myself.

@Nordgaren
Copy link
Author

You are compiling with gcc toolchain on linux, I assume? Only thing I can think of why we are getting different behavior.

@Speykious
Copy link
Owner

We're just compiling with Rust's official stable toolchain

@Nordgaren
Copy link
Author

I am not sure exactly what it is. I hate it, for sure. It's definitely UAF and UB. I just don't think it's a buffer overflow, even though I would love it to be. We were just talking about this. I wanted to make a CTF with a buffer overflow in Rust.

@Creative0708
Copy link
Collaborator

I agree that it's not much of a buffer overflow, @Bright-Shard may I modify it to be a more traditional stack-based buffer overflow like that of one written in C?

@Speykious
Copy link
Owner

Speykious commented Feb 18, 2024

The results I get definitely indicate that this is a buffer overflow. We are indeed overflowing outside of the allocation of the new, smaller buffer, by having the reference to the old buffer point to the new buffer instead thanks to them having been placed at the same position on the stack.
(For reference, I use Arch Linux.)

cve on  main [?] is 📦 v0.3.0 via 🦀 v1.76.0 
❯ cargo run bo        
    Finished dev [unoptimized] target(s) in 0.00s
     Running `target/debug/cve-rs bo`
Source code for this file is available here: https://github.com/Speykious/cve-rs/blob/main/src/buffer_overflow.rs
        
Before write:
New buffer (small): [1, 1, 1, 1, 1]
Old buffer (large): [1, 1, 1, 1, 1, 0, 0, 0, 0, 0]
After write:
New buffer (small): [3, 3, 3, 3, 3]
Old buffer (large): [3, 3, 3, 3, 3, 3, 3, 3, 3, 3]

I think the one way the results could be different is if the hack of having the two boxes at the same place in the stack is actually not reliable.

@Bright-Shard
Copy link
Collaborator

Why this is a buffer overflow

Exactly what Creative said. I don't care if the boxes point to the same place in the heap or not. We're just dealing with the stack. I'm going to go through a whole-ass breakdown here to hopefully clarify this once and for all. But to be clear, we do not care where the boxes' buffers get allocated to.

I think the box is confusing here because it is used to refer to data in the heap, but there's really 2 things at play there - a pointer and a buffer. So I'm going to stop referring to boxes. I'm going to refer to pointers, which are on the stack and point to some allocated memory in the heap, and buffers, which are the actual data in the heap. Finally, I'll refer to the reference, which is our borrow of the original box and refers to the pointer.

We first allocate a 10-byte buffer in memory and create a pointer to it. We create a static reference to that pointer, but then drop the pointer.

TO BE CLEAR: Neither the 10-byte buffer nor the pointer to that memory exist anymore. They are both gone. However, our reference still assumes the pointer exists, and we still have that reference. Furthermore, the reference assumes that it refers to a pointer that is pointing to a 10-byte buffer.

What happens next is the most crucial, and least stable, part of the exploit. We allocate a new buffer in memory, which is this time 5 bytes in size. The pointer to that buffer is then pushed to the stack. Because the old pointer has been removed from the stack, and the new pointer has been added to it, the new pointer effectively overwrites the old pointer on the stack. The reference that we had before is now referring to the new pointer, the one to the 5-byte buffer, because the old pointer has been overwritten by it.

We now use the reference to write to our buffer. Let's clarify what the reference thinks it is doing, vs what it is actually doing, at this point:

  • The reference thinks it is borrowing a pointer to a 10-byte buffer in memory. The reference is not aware that neither the pointer nor buffer exist anymore - it's just borrowing a location on the stack.
  • The reference is actually borrowing our new pointer to a 5-byte buffer in memory, because that pointer has been put in the same place on the stack that the old pointer used to live at, when it still existed.

So now, when we fill our buffer with the reference, Rust thinks we are accessing a pointer to a 10-byte buffer, and thus writing 10 bytes to a 10-byte buffer, which is safe. In reality, we are accessing a pointer to a 5-byte buffer, and thus writing 10 bytes to a 5 byte buffer, which is a buffer overflow.

Why this isn't working for you

I'm inclined to agree it's platform difference. I believe all 3 of us are on Arch Linux, and so the generated binary probably behaves differently.

What's probably happening is that the pointers are not getting put in the same place on the stack. So when it goes to read the buffer from that pointer, it just reads garbage on the stack, not an actual pointer - resulting in either nonsensical printouts or an access violation.

Maybe try running this in WSL?

@Nordgaren
Copy link
Author

The results I get definitely indicate that this is a buffer overflow. (For reference, I use Arch Linux.)

cve on  main [?] is 📦 v0.3.0 via 🦀 v1.76.0 
❯ cargo run bo        
    Finished dev [unoptimized] target(s) in 0.00s
     Running `target/debug/cve-rs bo`
Source code for this file is available here: https://github.com/Speykious/cve-rs/blob/main/src/buffer_overflow.rs
        
Before write:
New buffer (small): [1, 1, 1, 1, 1]
Old buffer (large): [1, 1, 1, 1, 1, 0, 0, 0, 0, 0]
After write:
New buffer (small): [3, 3, 3, 3, 3]
Old buffer (large): [3, 3, 3, 3, 3, 3, 3, 3, 3, 3]

I think the one way the results could be different is if the hack of having the two boxes at the same place in the stack is actually not reliable.

It's going to be very dependent on a lot of things, possibly. Rust doesn't even guarantee to generate the same machine code for the same crate in two different programs or on two different versions of the same compiler, and I have tested this with bindiff, and the same binary compiled on two different compiler versions generated a lot of different machine code.

Like I said, it has the potential to overflow the buffer, but it's not in itself a buffer overflow. You do need to overflow the target buffer for it to be a buffer overflow.

These aren't being place on the same spot on the stack in my environment, which is why it's not generating the same output. In no circumstances could I get it to overflow, and a colleague of mine did this on a Linux machine as well, although IDK if arch, and my only arch box is like 1.5 years old.

I would bug report this as UAF, undefined behavior with safe Rust, and a potential to generate a buffer overflow noting the environment which the bug showed up. In any event, I think this bug has been around for a long time and they still haven't fixed it, and that is sad.

I would love to see something that can reliably generate a buffer overflow. Stack or heap based.

@Speykious
Copy link
Owner

Speykious commented Feb 18, 2024

Yeah, it's definitely not the most reliable thing... Rust's stack unpredictability makes this difficult and I think I'm convinced at this point that this is the only point that could make this fail.

@Creative0708 decided to start making a new buffer overflow example, we'll see how that goes. It'll probably be stack-based because it'll let us overflow a buffer that will corrupt some other data on the stack. I just don't know if we'll be able to do anything about it not working as expected on some platforms machines, given that the stack seems to be the problem in the first place u_u

@Nordgaren
Copy link
Author

Yeah, it's definitely not the most reliable thing... Rust's stack unpredictability makes this difficult and I think I'm convinced at this point that this is the only point that could make this fail.

@Creative0708 decided to start making a new buffer overflow example, we'll see how that goes. It'll probably be stack-based because it'll let us overflow a buffer that will corrupt some other data on the stack. I just don't know if we'll be able to do anything about it not working as expected on some platforms, given that the stack seems to be the problem in the first place u_u

My colleague is telling me the unreliability comes from it being a kernel issue. I am not really a Linux dev, but he says it's because Arch sets up the stack differently.

@Nordgaren
Copy link
Author

Anyways, sorry for busting your balls. Still love what you guys are doing!

@Bright-Shard
Copy link
Collaborator

This is a heap-based overflow. It's not reliable across platforms, I guess.

We tried a stack-based overflow with slices but couldn't get them to align so it didn't ever work out. Hopefully creative's new attempt will and then we can have a heap-based and stack-based.

@Bright-Shard
Copy link
Collaborator

Bright-Shard commented Feb 18, 2024

Like I said, it has the potential to overflow the buffer, but it's not in itself a buffer overflow. You do need to overflow the target buffer for it to be a buffer overflow.

I don't understand what's not getting across here. We're writing 10 bytes to a 5 byte buffer using the pointer swap I described above. How is that not an overflow?

I'm being told I misunderstood your statement, sorry xD

@Speykious
Copy link
Owner

Anyways, sorry for busting your balls. Still love what you guys are doing!

Haha don't worry it's fine. We were committed to the joke being accurate anyways. xD

@Speykious
Copy link
Owner

Speykious commented Feb 18, 2024

Like I said, it has the potential to overflow the buffer, but it's not in itself a buffer overflow. You do need to overflow the target buffer for it to be a buffer overflow.

I don't understand what's not getting across here. We're writing 10 bytes to a 5 byte buffer using the pointer swap I described above. How is that not an overflow?

Basically the gist is that it's only a buffer overflow on specific systems where the stack has the reliability we need. And well, apparently...

I am not really a Linux dev, but he says it's because Arch sets up the stack differently.

I didn't even know the Linux kernel could do this, but apparently it can. Oh well. 💀

@Nordgaren
Copy link
Author

also, this might help a bit with debugging, if you increase the length of buffer_ref to something above 32, it should break this example even on Arch.

@Bright-Shard
Copy link
Collaborator

yeah, you're right, it does... not sure why atm, I'll have to look into that.

Right now we're trying to make transmute more stable and then I think we're going to redo this with transmute instead of the current code.

@Nordgaren
Copy link
Author

Nordgaren commented Feb 18, 2024

yeah, you're right, it does... not sure why atm, I'll have to look into that.

Right now we're trying to make transmute more stable and then I think we're going to redo this with transmute instead of the current code.

It's because the allocator will choose to make a new allocation for the greater than 32 byte sized chunk, rather than the smaller than 32 byte sized chunk. This is a caching optimization, I believe.

Edit: see tcache bin for glibc

@Bright-Shard
Copy link
Collaborator

But this should just rely on the pointer to the allocation, not the size of the allocation, that's what I don't understand. Unless it's making the stack more convoluted with more function calls or something

@Nordgaren
Copy link
Author

You might want to check a disassembly of the compiled output. You may find that you aren't actually using the same spot on the stack. That is what I had found. I can actually get the overflow to work if I mess with the stack a bit in the code, on my end. For example, it will overflow if I use a hex formatter for anything, instead. I think this is because it is coincidentally writing the new buffer pointer to the place on the stack where the reference is pointing to, or something.

Rust machine code generation is incredibly verbose, and the compiler will also optimize out fat pointers and in line sizes everywhere, and it gets really hard to track things down. I still gotta look in x96 dbg, and I also haven't looked at the code generated from the hex formatter trick. As I mentioned above, I have been doing some research on Rust code generation, and have found that Rusts code generation is pretty unreliable, and the compiler will make wildly different decisions sometimes based off the slightes difference in code. You might even find that the Rust compiler generated something different for the bigger buffer, too, in addition to the TCache optimization.

@Creative0708
Copy link
Collaborator

Fixed in f09d049. Have fun!

@Bright-Shard
Copy link
Collaborator

well it worked for him but not me. the stack is remarkably unstable.

I'm gonna reopen this, and let's keep it open until it works for everyone (including nordgaren)

@Bright-Shard Bright-Shard reopened this Feb 18, 2024
@Nordgaren
Copy link
Author

Nordgaren commented Feb 18, 2024

Yea. Also didn't work for me. I tried just transmuting an array and another array, but no luck.

	let password = black_box([0u8; 8]);

	// String is a Vec<u8> which is a (RawVec, usize) which is a ((Unique, usize), usize)
	// Rust explicitly says that structs are not guaranteed to have members in order
	// but this is sketchy enough anyways so :shrug:
	let mut name = black_box(crate::transmute::<_, [u8;16]>(password));
	println!("{password:?}");

	for i in 0..name.len() {
		name[i] = 1;
	}
	println!("{password:?}");

in this example, Rust still makes two different arrays.

@Bright-Shard
Copy link
Collaborator

Bright-Shard commented Feb 18, 2024

We're really just going to be at war with compiler optmisations here... and I don't think there's any way to completely turn them off. Creative found that the arrays were getting allocated the wrong way (ie password allocates before name), and I've not had any luck trying to write to the buffer manually. I think transmute isn't quite working as intended.

I'm going to bed but will try to look into it more tomorrow. By the way, how are you managing to view the assembly? I'm just opening mine in binja but I can't find the functions I want to, and even with no_mangle it doesn't appear in the symbol table.

Edit: Yeah transmute appears copying the data, not modifiying it. I have this code:

#[no_mangle]
#[allow(clippy::pedantic)]
pub fn buffer_overflow() {
	let name_buf = black_box([0u8; 16]);

	let zero = black_box(name_buf[0] == 0);
	let password = if zero {
		black_box([0u8; 7])
	} else {
		panic!("Dude wtf");
	};

	let mut name: [u8; 24] = transmute(name_buf);
	let mut stdin_buffer = [0];
	let mut idx = 0;
	while stdin_buffer[0] != b'\n' {
		stdin().read_exact(&mut stdin_buffer).unwrap();
		name[idx] = stdin_buffer[0];
		idx += 1;
		println!("Read {idx}");
		if idx > 24 {
			println!("Hold it right there, partner! That name's too large for these parts.");
		}
	}

	println!("name: {:?}", name_buf);
	println!("password: {:?}", password);
	println!("transmuted name: {:?}", name);
}

...and got this output:
image

So the first two buffers aren't getting modified, but the transmuted one is.

@Speykious
Copy link
Owner

We refactored the buffer-overflow example to be a little password overflow thing.

I managed to write a small function that determines the layout of a String at runtime so that we can safely transmute to it. I think it should be way more reliable now!

6c91b10

@Nordgaren Could you try this new refactor?

@Speykious
Copy link
Owner

(ok it currently segfaults in release. I don't know why but at least it's gonna be reliable in debug mode.)

@Nordgaren
Copy link
Author

In debug mode it just tells me I didn't modify the password, so that buffer is still all 0s

In release mode, I get

thread 'main' panicked at src\transmute.rs:34:22:
called `Option::unwrap()` on a `None` value

It's funny, when I woke up, I was like "Maybe I should suggest trying to use mem::forget" but it seems you guys are already on it. :P

@Speykious
Copy link
Owner

In debug mode it just tells me I didn't modify the password, so that buffer is still all 0s

And you tried with AAAAAAAAAAAAAAAAletmein! as input? That's weird...

Interesting that you get that in release mode, for us it's immediate segfault lol. Oh well.

@Nordgaren
Copy link
Author

Ah, yea. It works. I tried using a long password at first, but I must have just typed in 16 chars exactly lol. Whoops

Yea, that seems to work

@Nordgaren
Copy link
Author

Maybe black box the function that gets the new buffer, and it won't break in release? I will try to look at a release build sometime this week. Hopefully in the next day or two.

@Creative0708
Copy link
Collaborator

In debug mode it just tells me I didn't modify the password, so that buffer is still all 0s

In release mode, I get

thread 'main' panicked at src\transmute.rs:34:22:
called `Option::unwrap()` on a `None` value

It's funny, when I woke up, I was like "Maybe I should suggest trying to use mem::forget" but it seems you guys are already on it. :P

Alright, I've completely redone the transmute function in d102be5 to rely on enum abuse instead of stack abuse, and it shouldn't panic now.

Could you see if the buffer overflow works now? Thanks

@Nordgaren
Copy link
Author

Works for me! Seems to also be reliable as a buffer overflow! Works on both debug and release builds!

Asked my colleague to test on his Linux machine as well. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants