tiffany352 / rink-rs

Unit conversion tool and library written in rust
https://rinkcalc.app/about
GNU General Public License v3.0
408 stars 28 forks source link

Raising to the nth power can quickly trigger RAM exhaustion #88

Open mkroman opened 3 years ago

mkroman commented 3 years ago

Initially found by @secretdk - when using a caret (^) to raise a number value to an nth power, it's easy to trigger memory exhaustion.

Example:

% rink '0x41414141 ^ 0x72727272'
… eventually dies because OOM

I'm only assuming that the reason for the memory exhaustion is because it's trying to raise a large number to a large power.

mkroman commented 3 years ago

I realize that putting an upper memory limit on something like this is not a trivial task, but mayhaps even using a memory allocator with a hard - configurable - allocation limit is one way to go about this, at least when used as a service?

tiffany352 commented 3 years ago

The old impl of rink-irc and rink-web had some Linux-only code to hard limit memory and execution time using setrlimit, I'd recommend doing that in this case. I haven't reimplemented that in the new version of rink-web yet though. I found 150 megabytes of virtual memory and 15 seconds of execution time was more than enough for most queries.

mkroman commented 3 years ago

The old impl of rink-irc and rink-web had some Linux-only code to hard limit memory and execution time using setrlimit, I'd recommend doing that in this case.

Ah, I see. I'll use that for now, then. There's also the option of wrapping the default allocator with something like cap.

I found 150 megabytes of virtual memory and 15 seconds of execution time was more than enough for most queries.

Oh? By most queries I guess that would mean “realistic” user queries? :smile:

I'm using setrlimit through the rlimit crate as such:

setrlimit(
    Resource::AS,
    Rlim::from_usize(150 * 1000 * 1000),
    Rlim::from_usize(200 * 1000 * 1000),
)
.expect("Could not set memory limit");

and something I'd use as a test string, like pi ^ pi is enough to exhaust that limit. I think I'll use 512 or 1024 MB for now. :stuck_out_tongue_closed_eyes:

Is it even feasible to write an API with a memory-tracking allocator that returns errors and unwinds properly, such that a single query can't exceed a configurable memory limit, and doesn't crash? I feel like memory allocation checks in Rust may be panics all the way down.

tiffany352 commented 3 years ago

Memory usage might be a little bit higher than it was back then, in that case. I remember the majority of the memory usage was the app's code and various system shared libraries mapped into memory, and very little was the actual heap.

Spawning a child process and having the child limit memory using cap seems like a viable cross-platform alternative to using setrlimit. CPU could be limited by a timer in the parent process that kills it if it takes too long. Last time I looked at this, Rink was using GMP bindings that didn't support changing the allocator, now that it uses num that should be no problem.

It seems like there are some crates that try to wrap this functionality up in a cross platform way, but none of them seem maintained, and not designed for this specific usecase. Like gaol: https://github.com/servo/gaol

tiffany352 commented 3 years ago

I've just merged new sandboxing code for this. Right now it's only used in the CLI, so I think I'll need to put in some more work to make it fully usable for service-based frontends like irc/web/matrix.

Can be enabled in the config like this:

[limits]
enabled = true
memory = "20MB"
timeout = "10s"