monotone-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Monotone-devel] making I(), E(), N() throw bad_decode for network data


From: Timothy Brownawell
Subject: [Monotone-devel] making I(), E(), N() throw bad_decode for network data
Date: Mon, 10 Nov 2008 04:26:59 +0000

It's possible to I() a server by sending invalid data, this actually
happened here when merge_into_dir came out and not-up-to-the-minute
versions thought the resulting revisions were bogus.

I've just pushed a revision that makes the sanity macros look at a
'made_from' value, and throw bad_decode instead of their normal behavior
if that value is set to made_from_network. ATOMIC and ENCODING types and
revision_t have a member like this, and
        revision_t rev;
        rev.made_from = made_from_network;
        read_revision(garbage_data, rev);
does actually throw bad_decode now for at least some kinds of garbage.

I don't particularly like having the macros take a "hidden" argument
like this, but I'm not sure what would be better. Having them call some
function that you can declare in your class seems slightly cleaner, but
that wouldn't allow things like
        made_from_t made_from(rev.made_from);
        I(rev.new_manifest == roster_manifest_id);
(database::put_roster_for_revision) and keeping the function call inside
the condition check (so arguments, especially the i18n_format arguments
to N and E, are only evaluated when needed) prevents doing
        rev.I(rev.new_manifest == roster_manifest_id);
. Does anyone know a way that would work better? Maybe we could pass the
made_from explicitly, but then there are plenty of cases where you just
don't have one.

This also treats checks of function-local logic (I(false) on
can't-get-here branches and such) the same as checks of data structures,
so they also throw bad_decode if the data structure the function is on
is marked as having come from the network. Do we want this? Is it worth
avoiding this? It seems wrong, but fixing it would make it easier to use
the wrong sanity macro and go back to throwing informative_failure on
evil network data, and this isn't the easiest thing to write tests
for...


-- 
Timothy

Free (experimental) public monotone hosting: http://mtn-host.prjek.net





reply via email to

[Prev in Thread] Current Thread [Next in Thread]