Re: Preventing Denial of Service Attack In IPC Serialization
On Jul 7, 4:48 pm, brang...@ntlworld.com (Dave Harris) wrote:
Hi,
Here's what I got out of the discussion. The core idea was that the
amount of memory allocated should be roughly proportional to the amount
of data sent over the link.
Given that, we can control the denial of service attacks by placing a
limit on the amount of data the link accepts.
This is true...but you do not want to do it the way you specify
below....
One way to do this is by using packets and buffers. It seems to me that
it could also be done by keeping a count of the number of bytes received
and checking that it never gets too big. This way would avoid the
overhead of extra data copies and memory allocations of a packet-oriented
approach. However the limit is enforced, it is a property or policy of
the socket, not of the generic serialisation framework.
Not true.
It will be eventually seen that the serialization framework must be a
first-class participant in limiting the amount of data received by the
socket. My gut feeling is that pure virtual functions implemented in
the Archive class are called for.
It will also be seen that, in the majority of cases, it is ideal to
let the object itself specify to the socket the limit on amount of
data received.
The responsibility of the serialisation framework is to ensure that the
core idea is true. It needs to avoid allocating more memory than the
socket is allowed to receive, and preferably not more memory than the
socket actually has received. The socket needs to make that information
available. So I'd expect code like:
size_t size;
ar >> size;
size_t limit = ar.bytes_allowed() / sizeof(T);
vec.reserve( min( size, limit ) ); for (size_t i = 0; i < size;
++i) {
vec.resize( i+1 );
ar >> vec.back();
}
My solution does not require any reallocation whatsoever.
We need a reasonable policy for bytes_allowed(). I think it should
probably return at least 1024 even if not that many bytes are available
in buffers yet. The bigger it is, the less reallocation and copying but
the more vulnerability to denial of service.
I understand what you are saying, but this is not really the issue.
You're talking about picking a buffer that is limited in size but big
enough to avoid reallocation....
Think about this for a second. This is not the issue. The problem
does not go away. The problem was that, if you have a generalized C++
std::string object, that is being serialized into, you do not want the
source of the serialization to induce the target to allocate a huge
amount of memory for that string. What goes on in between the time
that the source exports the string with >> and the target imports the
string with >> is almost irrelevant. In the end, if the target has
indicated that, "I shall now send to you a string that is 16MB long,",
the source will eventually have in its RAM a 16MB string. That is the
issue.
It should be intuitively obvious that the socket, while *EXTREMELY*
capable of limiting how many bytes is received by it, will not have a
clue what that limit should be. Only the context of the application,
and specially, the point of serialization of objects, will know what
that limit should be. That's why I proposed my stack solution.
Given a reasonably large bytes_allowed(), many vectors will not need to
reallocate and copy their memory at all, because their final size will be
small enough. Bigger ones will have some overhead, but it will be limited
by their logarithmic growth policy and because they are given a
reasonably large capacity to start with.
Yes, and one 16MB string will DoS my PDA.
This approach is orthogonal to your push/pop_limit scheme. Both can
co-exist. One problem I have with your scheme is that it says, in effect,
that one data structure is allowed to allocate more memory than another.
This is wrong because it should not be a property of the data structure,
but of the socket. A denial-of-service attacker will seek out the trusted
data structures to exploit.
I have, as of July 1, 2007, decided that all of my data structures
will indicate to the "Archive" from which they are about to construct
themselves as a matter of serialization will indicate, if appropriate,
limits on how much data the Archive is allowed to generate. I have
not written any code, but in my mind, at least this part of my
solution is highly regular.
It also quite low-level. It requires us to count up the number of bytes
each structure might need. But we don't actually care about this. We want
to limit the /total/ amount of memory allocated across /all/ the data
structures, not the amount used by any specific one of them. It's at the
wrong level of abstraction.
My stack-based scheme takes the total amount of memory into account
to. Sorry, don't have the link handy, but if you read it again, you
will see that, when pop_limit() is invoked against the Archive, it
decimates the limit that is to be next top-of-stack (TOS). In this
case, complex data structures with arbitrary nesting can be safely
serialized. The only problem that I see with my method, which I am
purposely avoiding discussion until we are all in agreement, is that
*sometimes* it will be necessary for the programmer using my library
of classes to explicitly invoke push_limit(), pop_limit() against the
Archive, say for List<Foo>.
This "drawback" will be true, in general, of container classes,
because such classes have no way of knowing, a priori, how big they
should be. And it would be imprudent from an engineering point of
view to pre-specify a limit, since 100 is no more valid than
100,000,000, generally speaking.
But, the push/pop nature of my solution will maintain regularity,
which is probably the most important objective.
If it has to be done at that level, I'd suggest having push_limit()
affect the result of bytes_allowed(). Pushing a big number would
temporarily increase efficiency (by reducing vector reallocations) at the
cost of increased vulnerability. That trade-off seems unavoidable to me.
To be honest, it seems like a hack; like saying we care more about
efficiency than security.
In my serialization framework, there is only one buffer associated
with a socket. Data comes into the buffer, and I serialize into
objects from that buffer with no problem. The limit on the size of
that buffer _never_ changes. Right now, it is 8192 bytes. As far as
efficiency goes, that number is quite sufficient. Note again,
tweaking the size of this buffer does not solve the problem I
mentioned in my OP.
If you use different limits for different structures the hackers will
seek out the structures with the biggest limits and focus their attacks
there, ignoring the rest. So you might as well give the rest the same
limit; you don't increase vulnerability by doing so, as long as the
socket itself limits the total number of bytes received.
Again, these buffers are not really related to the problem.
My impression is that you have picked up on inefficiencies of specific
implementations. I have tried above to emphasis the core ideas, and show
how different implementations can reduce or avoid inefficiencies.
No, its not a problem for inefficiency. Those frameworks, including
Boost, are fundamentally defective. I want to emphasize that my own
framework was just as defective until I changed it recently (in last 4
days). Fiddling with socket buffers or pre-allocated buffer to
"serialize from" piece by piece does not solve the problem.
My stack solution, so far, is the only real solution to the problem.
Another variation, if you are still bothered by the cost of growing
vectors incrementally, is to replace the vector code with something like:
size_t size;
ar >> size;
if (size > ar.bytes_allowed() / sizeof(T))
throw "too big";
vec.resize( size );
for (size_t i = 0; i < size; ++i)
ar >> vec[i];
Now the $1,000,000USD question: "What is the value of size?"
This treats bytes_allowed() as a hard limit. It seems to me it is less
flexible without being significantly more efficient or more secure, for a
given result from bytes_allowed(). (In practice we will probably need to
compensate for the loss of flexibility by increasing bytes_allowed(), so
it will end up being less secure.)
Right. This code in a library would be intolerable (this should be
obvious). This code in the nude would be tedious, and the problem
would still exist for each T element of the vector.
-Le Chaud Lapin-
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]