From d5b4d6785879f0a5192c26f5b5e218aec8104798 Mon Sep 17 00:00:00 2001 From: Christophe Romain Date: Wed, 21 Sep 2011 14:39:50 +0200 Subject: [PATCH] Fix Denial of Service when user sends malformed publish stanza (thanks to Oleg Smirnov) (EJAB-1498) --- src/mod_pubsub/mod_pubsub.erl | 6 ++-- src/mod_pubsub/mod_pubsub_odbc.erl | 6 ++-- src/mod_pubsub/pubsub_odbc.patch | 46 +++++++++++++++--------------- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl index 705c6e67a..d7643c21c 100644 --- a/src/mod_pubsub/mod_pubsub.erl +++ b/src/mod_pubsub/mod_pubsub.erl @@ -2108,8 +2108,10 @@ publish_item(Host, ServerHost, Node, Publisher, ItemId, Payload) -> case lists:member("auto-create", features(Type)) of true -> case create_node(Host, ServerHost, Node, Publisher, Type) of - {result, _} -> - publish_item(Host, ServerHost, Node, Publisher, ItemId, Payload); + {result, [{xmlelement, "pubsub", [{"xmlns", ?NS_PUBSUB}], + [{xmlelement, "create", [{"node", NewNode}], []}]}]} -> + publish_item(Host, ServerHost, list_to_binary(NewNode), + Publisher, ItemId, Payload); _ -> {error, ?ERR_ITEM_NOT_FOUND} end; diff --git a/src/mod_pubsub/mod_pubsub_odbc.erl b/src/mod_pubsub/mod_pubsub_odbc.erl index 9c24456cc..b8cf4899b 100644 --- a/src/mod_pubsub/mod_pubsub_odbc.erl +++ b/src/mod_pubsub/mod_pubsub_odbc.erl @@ -1921,8 +1921,10 @@ publish_item(Host, ServerHost, Node, Publisher, ItemId, Payload) -> case lists:member("auto-create", features(Type)) of true -> case create_node(Host, ServerHost, Node, Publisher, Type) of - {result, _} -> - publish_item(Host, ServerHost, Node, Publisher, ItemId, Payload); + {result, [{xmlelement, "pubsub", [{"xmlns", ?NS_PUBSUB}], + [{xmlelement, "create", [{"node", NewNode}], []}]}]} -> + publish_item(Host, ServerHost, list_to_binary(NewNode), + Publisher, ItemId, Payload); _ -> {error, ?ERR_ITEM_NOT_FOUND} end; diff --git a/src/mod_pubsub/pubsub_odbc.patch b/src/mod_pubsub/pubsub_odbc.patch index ef2ba0eee..cfa58640f 100644 --- a/src/mod_pubsub/pubsub_odbc.patch +++ b/src/mod_pubsub/pubsub_odbc.patch @@ -1,5 +1,5 @@ ---- mod_pubsub.erl 2011-08-31 16:08:42.000000000 +0200 -+++ mod_pubsub_odbc.erl 2011-08-31 16:08:42.000000000 +0200 +--- mod_pubsub.erl 2011-09-21 14:37:16.000000000 +0200 ++++ mod_pubsub_odbc.erl 2011-09-21 14:37:36.000000000 +0200 @@ -42,7 +42,7 @@ %%% 6.2.3.1, 6.2.3.5, and 6.3. For information on subscription leases see %%% XEP-0060 section 12.18. @@ -554,7 +554,7 @@ case Result of default -> {result, Reply}; _ -> {result, Result} -@@ -2241,7 +2054,7 @@ +@@ -2243,7 +2056,7 @@ %%

The permission are not checked in this function.

%% @todo We probably need to check that the user doing the query has the right %% to read the items. @@ -563,7 +563,7 @@ MaxItems = if SMaxItems == "" -> get_max_items_node(Host); -@@ -2255,12 +2068,13 @@ +@@ -2257,12 +2070,13 @@ {error, Error} -> {error, Error}; _ -> @@ -578,7 +578,7 @@ {PresenceSubscription, RosterGroup} = get_presence_and_roster_permissions(Host, From, Owners, AccessModel, AllowedGroups), if not RetreiveFeature -> -@@ -2273,11 +2087,11 @@ +@@ -2275,11 +2089,11 @@ node_call(Type, get_items, [NodeId, From, AccessModel, PresenceSubscription, RosterGroup, @@ -592,7 +592,7 @@ SendItems = case ItemIDs of [] -> Items; -@@ -2290,7 +2104,8 @@ +@@ -2292,7 +2106,8 @@ %% number of items sent to MaxItems: {result, [{xmlelement, "pubsub", [{"xmlns", ?NS_PUBSUB}], [{xmlelement, "items", nodeAttr(Node), @@ -602,7 +602,7 @@ Error -> Error end -@@ -2312,10 +2127,15 @@ +@@ -2314,10 +2129,15 @@ Error -> Error end. get_allowed_items_call(Host, NodeIdx, From, Type, Options, Owners) -> @@ -619,7 +619,7 @@ %% @spec (Host, Node, NodeId, Type, LJID, Number) -> any() -@@ -2327,31 +2147,29 @@ +@@ -2329,31 +2149,29 @@ %% Number = last | integer() %% @doc

Resend the items of a node to the user.

%% @todo use cache-last-item feature @@ -670,7 +670,7 @@ ToSend = case node_action(Host, Type, get_items, [NodeId, LJID]) of {result, []} -> []; -@@ -2374,20 +2192,7 @@ +@@ -2376,20 +2194,7 @@ [{xmlelement, "items", nodeAttr(Node), itemsEls(ToSend)}]) end, @@ -692,7 +692,7 @@ %% @spec (Host, JID, Plugins) -> {error, Reason} | {result, Response} %% Host = host() -@@ -2489,7 +2294,8 @@ +@@ -2491,7 +2296,8 @@ error -> {error, ?ERR_BAD_REQUEST}; _ -> @@ -702,7 +702,7 @@ case lists:member(Owner, Owners) of true -> OwnerJID = jlib:make_jid(Owner), -@@ -2499,24 +2305,7 @@ +@@ -2501,24 +2307,7 @@ end, lists:foreach( fun({JID, Affiliation}) -> @@ -728,7 +728,7 @@ end, FilteredEntities), {result, []}; _ -> -@@ -2569,11 +2358,11 @@ +@@ -2571,11 +2360,11 @@ end. read_sub(Subscriber, Node, NodeID, SubID, Lang) -> @@ -742,7 +742,7 @@ OptionsEl = {xmlelement, "options", [{"jid", jlib:jid_to_string(Subscriber)}, {"subid", SubID}|nodeAttr(Node)], [XdataEl]}, -@@ -2599,7 +2388,7 @@ +@@ -2601,7 +2390,7 @@ end. set_options_helper(Configuration, JID, NodeID, SubID, Type) -> @@ -751,7 +751,7 @@ {result, GoodSubOpts} -> GoodSubOpts; _ -> invalid end, -@@ -2628,7 +2417,7 @@ +@@ -2630,7 +2419,7 @@ write_sub(_Subscriber, _NodeID, _SubID, invalid) -> {error, extended_error(?ERR_BAD_REQUEST, "invalid-options")}; write_sub(Subscriber, NodeID, SubID, Options) -> @@ -760,7 +760,7 @@ {error, notfound} -> {error, extended_error(?ERR_NOT_ACCEPTABLE, "invalid-subid")}; {result, _} -> -@@ -2796,8 +2585,8 @@ +@@ -2798,8 +2587,8 @@ {"subscription", subscription_to_string(Sub)} | nodeAttr(Node)], []}]}]}, ejabberd_router:route(service_jid(Host), jlib:make_jid(JID), Stanza) end, @@ -771,7 +771,7 @@ true -> Result = lists:foldl(fun({JID, Subscription, SubId}, Acc) -> -@@ -3152,7 +2941,7 @@ +@@ -3154,7 +2943,7 @@ {Depth, [{N, get_node_subs(N)} || N <- Nodes]} end, tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]))} end, @@ -780,7 +780,7 @@ {result, CollSubs} -> CollSubs; _ -> [] end. -@@ -3166,9 +2955,9 @@ +@@ -3168,9 +2957,9 @@ get_options_for_subs(NodeID, Subs) -> lists:foldl(fun({JID, subscribed, SubID}, Acc) -> @@ -792,7 +792,7 @@ _ -> Acc end; (_, Acc) -> -@@ -3357,6 +3146,30 @@ +@@ -3359,6 +3148,30 @@ Result end. @@ -823,7 +823,7 @@ %% @spec (Host, Options) -> MaxItems %% Host = host() %% Options = [Option] -@@ -3753,7 +3566,13 @@ +@@ -3755,7 +3568,13 @@ tree_action(Host, Function, Args) -> ?DEBUG("tree_action ~p ~p ~p",[Host,Function,Args]), Fun = fun() -> tree_call(Host, Function, Args) end, @@ -838,7 +838,7 @@ %% @doc

node plugin call.

node_call(Type, Function, Args) -> -@@ -3773,13 +3592,13 @@ +@@ -3775,13 +3594,13 @@ node_action(Host, Type, Function, Args) -> ?DEBUG("node_action ~p ~p ~p ~p",[Host,Type,Function,Args]), @@ -854,7 +854,7 @@ case tree_call(Host, get_node, [Host, Node]) of N when is_record(N, pubsub_node) -> case Action(N) of -@@ -3791,13 +3610,19 @@ +@@ -3793,13 +3612,19 @@ Error end end, Trans). @@ -878,7 +878,7 @@ {result, Result} -> {result, Result}; {error, Error} -> {error, Error}; {atomic, {result, Result}} -> {result, Result}; -@@ -3805,6 +3630,15 @@ +@@ -3807,6 +3632,15 @@ {aborted, Reason} -> ?ERROR_MSG("transaction return internal error: ~p~n", [{aborted, Reason}]), {error, ?ERR_INTERNAL_SERVER_ERROR}; @@ -894,7 +894,7 @@ {'EXIT', Reason} -> ?ERROR_MSG("transaction return internal error: ~p~n", [{'EXIT', Reason}]), {error, ?ERR_INTERNAL_SERVER_ERROR}; -@@ -3813,6 +3647,17 @@ +@@ -3815,6 +3649,17 @@ {error, ?ERR_INTERNAL_SERVER_ERROR} end.