On Thu, 28 Jun 2007 c...@mailvault.com wrote:

So it might be nice if the serialization framework supported marking a
source as trusted and would use the non-paranoid version then. It's not
essential, though.

Sebastian Redl

You're beating a dead horse.

Jarl.

Might be better to have all or nothing.

-Le Chaud Lapin-

-Le Chaud Lapin-

Brian
Ebenezer Enterprises

B* b = new D(...);

could behave like usual and

B* b = new preserve D(...);

could return an I object if a D is not possible.  That
seems better than what I wrote above as it could be
applied to more than stream constructors.

Brian
Ebenezer Enterprises

There are two things:

#1) Even though it deserializes from a string, this specific example
does have multiple DoS bugs in it. handle_read_header() blindly
resizes inbound_data_ to whatever the headers tells it to. Yes,
inbound_data_ is then used to create a string from the data, but the
damage is done. Another DoS bug is in line 25 and 26 of the file
"stock.hpp" - It calls the default deserialization function for
std::string, which itself has the same problem - in boost/
serialization/collections_load_imp.hpp in the function
rebuild_collection().

#2) In my opinion, this specific example is not a very good example of
receiving data over a socket.

I say #2 because although one can put an arbitrary limit on
inbound_data_ which would solve the bug/DoS, this limit really would
be arbitrary.

Others in this discussion mentioned "1 megabyte".  What specific value
to use is not really known.

Yes, you can say that the 'application writer' can decide this. But
the real problem is that handle_read_header() is itself a template.
This method gets called regardless of whatever type is specified in
the original call to async_read(), therefore the same limit must be
used for all types T. Is the same limit reasonable regardless of
whatever object class is being deserialized?

I feel that this design of the serialization library is ultimately
flawed.

The flaw exists even if you are reading from a file or a fixed size
std::string instead of a socket.

The flaw forbids the application from controlling the limits for
individual fields in an object which are reasonably implemented as stl
containers or strings.

I guess it could be said that the concept of the flaw comes from the
fact that instantiated STL containers in general can not be given a
maximum size limit.. static STL allocators don't cut it for this
purpose - perhaps stateful allocators could help out if they became
more mainstream, see http://www.lancediduck.com/papers/Cpp/StatefulSTL.pdf

In the boost library, rebuild_collection() (in
collections_load_imp.hpp ) could be modified to take an actual object
that provides the function of the "class R" template parameter which
is the 'reserve_imp' functor. This would require changes to 'load()'
and the fancy >> and & operators...

However, this would allow the application code to specify limits on a
per-item basis.  Right now, rebuild_collection() and therefore most
deserializations, provide DoS bugs for your application code....

Here is where my personal philosphy comes into play:

Yes, I know it is safe if you 'trust' your data...
However 'gets()' is safe if you trust your data too, yet gets() is
deprecated and everybody knows to not use it!

--jeffk++

[Note to mods: I am resending this message because the original
message was apparently lost.]

I am going to show in the Boost serialization framework where the
author of that framework is doing almost exactly what I pointed out as
a DoS vulnerability in my original post. But first, to restate the
most serious problem briefly, with current serialization frameworks,
including MFC, Boost, and my own, and probably many others, without a
fundamental change in the model, the sender of a serialized object can
artificially induce the receiver to allocate a large amount of memory,
creating a denial-of-service attack. It should be noted that there are
other problems beyond the one I am about to demonstrate that would
arise without a fundamental change in these serialization frameworks,
but I choose this problem because it illustrates the general principle
quickly and does not require much insight into serialization to
recognize as faulty.

At the sending end of a connection, a programmer would write something
like:

int main ()
{
  Socket s;
  std::vector<int> v;
  s << v;
  return 0;

int main ()
{
  Socket s;
  std::vector<int> v;
  s >> v;
  return 0;

This second main() function, in the line s >> v, is where the problem
occurs. The essence of the problem can be seen in pieces of the
serialization framework, ignoring non-essential elements.  We will
dissect the framework, starting from the bottom  up, with a simple
piece.

In File http://www.boost.org/boost/serialization/collections_load_imp.hpp,
one will see:

template<class Container>
class reserve_imp
{
public:
    void operator()(Container &s, unsigned int count) const {
        s.reserve(count);
    }

In File http://www.boost.org/boost/serialization/collections_load_imp.hpp,
one will see:

template<class Archive, class Container, class InputFunction, class R>
inline void load_collection(Archive & ar, Container &s)
{
    s.clear();
    // retrieve number of elements
    unsigned int count;
    unsigned int item_version(0);
    ar >> BOOST_SERIALIZATION_NVP(count);
    if(3 < ar.get_library_version()){
        ar >> BOOST_SERIALIZATION_NVP(item_version);
    }
    R rx;
    rx(s, count); // <----- THIS IS THE PROBLEM. -Le Chaud Lapin-
    InputFunction ifunc;
    while(count-- > 0){
        ifunc(ar, s, item_version);
    }

1. empties the container with s.clear()
2. declares an unsigned int "count"
3. declares a version number
4. Reads "count", taking care that the "stuff" read in includes not
only "count", but the literal word 'count'
5. If library version is less than 3, then read in the item version
6. Instantiates a "reserve_imp" object, "rx".
7. Invokes operator () against rx, supplying the container and count
as arguments. << ***THIS IS THE PROBLEM***
8. Declares input function object.
9. Reads in each element of container "count" times using input
function.

It can be seen that the size of a std::vector<> will be sized by rx
arbitrarily to what the sender says the vector size should be (count),
even 50MB.  It does not matter whether the count is stored in a tiny
32-byte buffer or a huge 1MB buffer.  The vector will still be
unwittingly resize to whatever the sender says the size should be,
which could happen if the sender is malicious.

-Le Chaud Lapin-

Brian Wood
Ebenezer Enterprises
www.webebenezer.net

-Le Chaud Lapin-

Solutions aside, do you or do you not agree that, today, in 2007, 4 of
July, Boost Serialization is doing what I said we should avoid in my
OP

Do you agree that  Boost Serialization is using an implementation that
is subject to DoS as I wrote in my original post?

There are many people who are reading these posts would like to know a
simple yes/now answer to this question, because they might alter their
inclination to use such a framework in the nude if the answer is
"yes".

-Le Chaud Lapin-

To summarize:

* Boost serialization includes problem code which is the foundation of
a DoS.

* This problem happens with all containers including std::list,
std::vector, std::string.

* It does not matter if you are deserializing from a string or a
socket, remote or local, behind a firewal or not, the problem still
exists in Boost.ser.

* It is impossible for any application code to 'work around' this
problem without modifying Boost.ser.

* The boost.asio examples using boost.ser contain the DoS problem and
can not be fixed without modifying Boost.ser.

* If this problem were fixed in Boost.ser then there would be no
problem with doing IPC with Boost.ser.

These above facts do not mean that doing IPC with serialization is
bad.

The above facts only mean that doing IPC with the unmodified boost.ser
library is bad, and that the boost.asio serialization example is not a
good example since it contains hidden DoS problems.

--jeffk++

1. I pointed out that there was a flaw in my serialization framework,
and probably others.
2. Jeff pointed out that the flaw exits in Boost.
3. You attempted to refute that the problem exists in Boost.
4. I showed prima facie evidence that it does exist in Boost.

So, after so many posts, we at least arrive at a simple conclusion.
The problem does exist, and no dead horses are being beaten.  Now, the
next step, is to find a solution, and not to kick at the one you
proposed, I have one gets more regular each day I think about it.

In any case, you should not claim that a problem does not exist when
it does.  There are people who read these posts who do not understand/
care about all the details.  They only want to know the end result:
does a problem exist or not.  With your posts, it could have easily
been determined by a casual reader that there was no problem, and such
a reader might have used Boost Serialization in the nude, leaving
ample opportunity to crash their computers at will.

Do you at least admit that the problem exists in Boost (and MFC for
that matter)?

-Le Chaud Lapin-

Brian Wood
Ebenezer Enterprises

It is hard for me to understand why you want to continue this
discussion. Or why the moderators allow you to rehash the same
content, over and over again. The premise in your OP, of the inutility
of C++ serialization frameworks in IPC applications, was as sweeping
as it was incorrect, and stemmed from your own use of "socket
archives". And now you are implying that your original post only
concerned an implementation detail of Boost.Serialization.

If this whole thread isn't a dead horse, I don't know what is.

Jarl.

As far as giving credit to Jarl...I am the last person to want to deny
credit to someone for having an idea, even one that (IMO) is only
good, but not great.  I was inclined last night to give Jarl credit
for his buffer technique...but after thinking more, I do not regard
this as a solution.  The truth is that I feel that the minimum
standard that I would hold for a serialization framework is violated
by Jarl's method.  In particular, the pre-allocation buffer is, IMO,
intolerable, not to mention the multiple-allocation methods that he
and others described.  His method would fail on a PDA with only 64MB
of RAM, and I do run my serialization code on PDA's, and so would a
few others.

I would also like to assert for the recorded that I am all but
convinced that the only survivable (not perfect, but workable)
_requires_ participation by the objects themselves to limit
resources.  Before I was only vaguely suspicious.  Now I am certain.
Jarl's method ignores participation by the objects.

Finally, I am _not_ proposing my method as a solution.  As I have said
many times before, my method, like Jarl's involves arbitrariness, and
arbitrariness (to this extent) is usually bad in engineering.

However, my method does not require pre-allocation of memory, nor does
it require superfluous reallocations why an object is being serialized
into.  Any arbitrariness that my method has, Jarl's method will
certainly have.

That is why I would find it hard to give him credit for the buffer
method.  I believe that my method, as weak as it is, is better than
what he proposed.

-Le Chaud Lapin-

The fact is, you tried to refute Jeff's assertion that Boost was doing
exactly what I wrote about in the OP, and when I demonstrated
indisputable evidence two threads up, you want to discontinue the
discussion.

If I had been a random casual reader of this thread who, contrary what
you wrote to your previous post, had not had any experience with
serialization, and I had decided that Le Chaud Lapin and Jeff
Koftinoff were wrong, and Jarl Linrud was right, I might have
continued using Boost under the impression that there was no problem,
which would have been a problem indeed.

At least Brian Wood had the sensibility that Boost Serialization has a
flaw in I pointed out, and because of that flaw, it is possible to to
crash machines all over the Internet that use Boost Serialization in
the nude.

When you present your arguments, the they are not just for our
benefit.  They are for the benefit of everyone who reads this group
and wants to gain insight to truth.  That is why it is so important to
see truth.

-Le Chaud Lapin-

1) The flaw in B.Ser was pointed out by Sebastian Redl, not you.
2) The simple fix for it was also pointed out by Sebastian Redl.
3) That simple fix is valid, as long as one only passes in bounded
amounts of data to the deserialization framework.

I think we can agree on those 3 points.

If you still think, as you did in your OP, that there is a general
problem with the use of C++ serialization frameworks in IPC
applications, then please specify exactly what that problem is.

Jarl.

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.

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.

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.

My stack solution, so far, is the only real solution to the problem.

-Le Chaud Lapin-

For example, if your platform is a "real-mode" OS without virtual
memory, the limit may ultimately be determined by the amount of physical
RAM available to your C-runtime heap.

Setting a limit on inbound_data_size doesn't solve the problem of an
attacker sending you bogus object deserialization requests, though.

Ethan Cohen