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]

(cherry picked from commit 7629209219bbea3f127b33be06ac427ebc3a559e)

Change-Id: I1726a834eb98b7e06285bac78a74e20bbedb9ce8

diff --git a/src/vlserver/vldbint.xg b/src/vlserver/vldbint.xg
index ff34d12..523fe80 100644 (file)
--- 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;