4 ms·
I suppose you could start each message with a magic constant, followed by the length of the remainder, then the payload. Any messages that don't start with the
by s_kilk 10y ago
I suppose you could start each message with a magic constant, followed by the length of the remainder, then the payload.
Any messages that don't start with the magic constant just get ignored.
- amelius 10y agoIt could help in obvious cases, but it sounds a lot like "security by obscurity".
- TillE 10y agoRight, it does nothing to protect from targeted DoS attacks. The obvious answer is that you usually want a fixed-size buffer per connection, and any message larger than that needs to be read and handled chunk by chunk.
- pdpi 10y agoNo, it's just a bit of defensive programming against silliness and misconfiguration. Not every errant connection is an attacker trying to pwn you, some are just going to be honest mistakes. Picking up on wrong magic numbers and logging "missing magic from <ip>" would help immensely debugging this issue. All the other advice about avoiding mallocing an unsanitised amount of memory still stands, but this would just make it all easier to figure out
- deleted 10y ago[deleted]
- phamilton 10y agoMost database drivers have a binary protocol that does this. It's a pretty standard practice. It's not for security, it's for protocol negotiation.
- jlarocco 10y agoI disagree. It's a well known method of sanity checking serialized data, and it's used by lots and lots of file formats and "over the wire" data formats. It's not really a security feature, and it doesn't have to be a secret value. It also doesn't mean you shouldn't sanity check the other data fields. The sole purpose is to quickly rule out data that's blatantly incorrect.
- dmitrygr 10y agoCool idea, BUT nobody uses SCTP (packet based). UDP is not reliable (most applications dislike this), and TCP does not have "messages" actually requiring you to send length in some form...sort of like these guys did...
- justinhj 10y agoUnless you have a fixed sized header which includes the size. That's all that is needed to avoid people having to pull up a debugger to figure out why things are going bad in production.
- kbenson 10y ago> Unless you have a fixed sized header which includes the size. Isn't that actually part the problem here? It's getting an erroneous size and trying to allocate a big buffer so it can read the data even though the data isn't really that big. One solution might be a fixed header size, and a header checksum. Allocate space for the header, read what should be the header, including the checksum, and if the checksum is correct, then allocate the space requested for the data. A fixed size header doesn't really help unless you are actually checking that it's valid before proceeding.
- caf 10y agoThe problem here was the memory leak, because it didn't free the enormous buffer in the failure path.
- kbenson 10y agoNo, the original post was about a memory leak. This is really just a follow on about finding weird values popping up in protocols. Also, even if we do constrain this to memory leaks, I would say that leaks just make the problem worse, and a real bad bug that causes a persistent DOS rather than just an ephemeral DOS, but it's still a problem without them. Actually allocating the memory prior to confirming you need it is bad because if you get enough requests that do that quick enough, you eat up all the memory. If you aren't freeing the memory, you just don't have to be as quick. Considering that that crazy value in question here that is being passed to malloc represents over 1GB of memory, it wouldn't take that many requests at all.