From 7629209219bbea3f127b33be06ac427ebc3a559e Mon Sep 17 00:00:00 2001 From: Mark Vitale Date: Thu, 5 Jul 2018 21:11:30 -0400 Subject: [PATCH] OPENAFS-SA-2018-003 vlserver: prevent unbounded input to VL_RegisterAddrs VL_RegisterAddrs is defined with an input argument of type bulkaddrs, which is defined to XDR as an unbounded array of afs_uint32 (IPv4 addresses): typedef afs_uint32 bulkaddrs<> The <> with no value instructs rxgen to build client and server stubs that allow for a maximum size of "~0u" or 0xFFFFFFFF. Ostensibly the bulkaddrs array is unbounded to allow it to be shared among VL_RegisterAddrs, VL_GetAddrs, and VL_GetAddrsU. The VL_GetAddrs* RPCs use bulkaddrs as an output array with a maximum size of MAXSERVERID (254). VL_RegisterAddrss uses bulkaddrs as an input array, with a nominal size of VL_MAXIPADDRS_PERMH (16). However, RPCs with unbounded array inputs are susceptible to remote denial-of-service attacks. That is, a malicious client may send a VL_RegisterAddrs request with an arbitrarily long array, forcing the vlserver to expend large amounts of network bandwidth, cpu cycles, and heap memory to unmarshal the argument. Even though VL_RegisterAddrs requires superuser authorization, this attack is exploitable by non-authorized actors because XDR unmarshalling happens long before any authorization checks can occur. Because all uses of the type that our implementation support have fixed bounds on valid data (whether input or output), apply an arbitrary implementation limit (larger than any valid structure would be), to prevent this class of attacks in the XDR decoder. [kaduk@mit.edu: limit the bulkaddrs type instead of introducing a new type] Change-Id: Ibcc962ccc46aec7552b86d1d9fda7cc14310bc03 --- src/vlserver/vldbint.xg | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vlserver/vldbint.xg b/src/vlserver/vldbint.xg index ff34d12..523fe80 100644 --- a/src/vlserver/vldbint.xg +++ b/src/vlserver/vldbint.xg @@ -208,7 +208,12 @@ const VLOP_DUMP = 0x100; typedef vldbentry bulkentries<>; typedef nvldbentry nbulkentries<>; typedef uvldbentry ubulkentries<>; -typedef afs_uint32 bulkaddrs<>; +/* + * 500 is an arbitrary implementation limit, larger than what we support storing. + * It lets the XDR decoder detect an attack (excessively large input) and reject + * it without incurring excessive resource usage. + */ +typedef afs_uint32 bulkaddrs<500>; struct VLCallBack { afs_uint32 CallBackVersion; -- 1.9.4