Brad Bishop | f8caae3 | 2019-03-25 13:13:56 -0400 | [diff] [blame^] | 1 | Description: sd-bus: enforce a size limit for dbus paths, and don't allocate |
| 2 | them on the stacka |
| 3 | Forwarded: no |
| 4 | |
| 5 | Patch from: systemd_239-7ubuntu10.8 |
| 6 | |
| 7 | For information see: |
| 8 | https://usn.ubuntu.com/3891-1/ |
| 9 | https://git.launchpad.net/ubuntu/+source/systemd/commit/?id=f8e75d5634904c8e672658856508c3a02f349adb |
| 10 | |
| 11 | CVE: CVE-2019-6454 |
| 12 | Upstream-Status: Backport |
| 13 | |
| 14 | Signed-off-by: George McCollister <george.mccollister@gmail.com> |
| 15 | |
| 16 | --- a/src/libsystemd/sd-bus/bus-internal.c |
| 17 | +++ b/src/libsystemd/sd-bus/bus-internal.c |
| 18 | @@ -45,7 +45,7 @@ |
| 19 | if (slash) |
| 20 | return false; |
| 21 | |
| 22 | - return true; |
| 23 | + return (q - p) <= BUS_PATH_SIZE_MAX; |
| 24 | } |
| 25 | |
| 26 | char* object_path_startswith(const char *a, const char *b) { |
| 27 | --- a/src/libsystemd/sd-bus/bus-internal.h |
| 28 | +++ b/src/libsystemd/sd-bus/bus-internal.h |
| 29 | @@ -333,6 +333,10 @@ |
| 30 | |
| 31 | #define BUS_MESSAGE_SIZE_MAX (128*1024*1024) |
| 32 | #define BUS_AUTH_SIZE_MAX (64*1024) |
| 33 | +/* Note that the D-Bus specification states that bus paths shall have no size limit. We enforce here one |
| 34 | + * anyway, since truly unbounded strings are a security problem. The limit we pick is relatively large however, |
| 35 | + * to not clash unnecessarily with real-life applications. */ |
| 36 | +#define BUS_PATH_SIZE_MAX (64*1024) |
| 37 | |
| 38 | #define BUS_CONTAINER_DEPTH 128 |
| 39 | |
| 40 | --- a/src/libsystemd/sd-bus/bus-objects.c |
| 41 | +++ b/src/libsystemd/sd-bus/bus-objects.c |
| 42 | @@ -1134,7 +1134,8 @@ |
| 43 | const char *path, |
| 44 | sd_bus_error *error) { |
| 45 | |
| 46 | - char *prefix; |
| 47 | + _cleanup_free_ char *prefix = NULL; |
| 48 | + size_t pl; |
| 49 | int r; |
| 50 | |
| 51 | assert(bus); |
| 52 | @@ -1150,7 +1151,12 @@ |
| 53 | return 0; |
| 54 | |
| 55 | /* Second, add fallback vtables registered for any of the prefixes */ |
| 56 | - prefix = alloca(strlen(path) + 1); |
| 57 | + pl = strlen(path); |
| 58 | + assert(pl <= BUS_PATH_SIZE_MAX); |
| 59 | + prefix = new(char, pl + 1); |
| 60 | + if (!prefix) |
| 61 | + return -ENOMEM; |
| 62 | + |
| 63 | OBJECT_PATH_FOREACH_PREFIX(prefix, path) { |
| 64 | r = object_manager_serialize_path(bus, reply, prefix, path, true, error); |
| 65 | if (r < 0) |
| 66 | @@ -1346,6 +1352,7 @@ |
| 67 | } |
| 68 | |
| 69 | int bus_process_object(sd_bus *bus, sd_bus_message *m) { |
| 70 | + _cleanup_free_ char *prefix = NULL; |
| 71 | int r; |
| 72 | size_t pl; |
| 73 | bool found_object = false; |
| 74 | @@ -1370,9 +1377,12 @@ |
| 75 | assert(m->member); |
| 76 | |
| 77 | pl = strlen(m->path); |
| 78 | - do { |
| 79 | - char prefix[pl+1]; |
| 80 | + assert(pl <= BUS_PATH_SIZE_MAX); |
| 81 | + prefix = new(char, pl + 1); |
| 82 | + if (!prefix) |
| 83 | + return -ENOMEM; |
| 84 | |
| 85 | + do { |
| 86 | bus->nodes_modified = false; |
| 87 | |
| 88 | r = object_find_and_run(bus, m, m->path, false, &found_object); |
| 89 | @@ -1499,9 +1509,15 @@ |
| 90 | |
| 91 | n = hashmap_get(bus->nodes, path); |
| 92 | if (!n) { |
| 93 | - char *prefix; |
| 94 | + _cleanup_free_ char *prefix = NULL; |
| 95 | + size_t pl; |
| 96 | + |
| 97 | + pl = strlen(path); |
| 98 | + assert(pl <= BUS_PATH_SIZE_MAX); |
| 99 | + prefix = new(char, pl + 1); |
| 100 | + if (!prefix) |
| 101 | + return -ENOMEM; |
| 102 | |
| 103 | - prefix = alloca(strlen(path) + 1); |
| 104 | OBJECT_PATH_FOREACH_PREFIX(prefix, path) { |
| 105 | n = hashmap_get(bus->nodes, prefix); |
| 106 | if (n) |
| 107 | @@ -2091,8 +2107,9 @@ |
| 108 | char **names) { |
| 109 | |
| 110 | BUS_DONT_DESTROY(bus); |
| 111 | + _cleanup_free_ char *prefix = NULL; |
| 112 | bool found_interface = false; |
| 113 | - char *prefix; |
| 114 | + size_t pl; |
| 115 | int r; |
| 116 | |
| 117 | assert_return(bus, -EINVAL); |
| 118 | @@ -2111,6 +2128,12 @@ |
| 119 | if (names && names[0] == NULL) |
| 120 | return 0; |
| 121 | |
| 122 | + pl = strlen(path); |
| 123 | + assert(pl <= BUS_PATH_SIZE_MAX); |
| 124 | + prefix = new(char, pl + 1); |
| 125 | + if (!prefix) |
| 126 | + return -ENOMEM; |
| 127 | + |
| 128 | do { |
| 129 | bus->nodes_modified = false; |
| 130 | |
| 131 | @@ -2120,7 +2143,6 @@ |
| 132 | if (bus->nodes_modified) |
| 133 | continue; |
| 134 | |
| 135 | - prefix = alloca(strlen(path) + 1); |
| 136 | OBJECT_PATH_FOREACH_PREFIX(prefix, path) { |
| 137 | r = emit_properties_changed_on_interface(bus, prefix, path, interface, true, &found_interface, names); |
| 138 | if (r != 0) |
| 139 | @@ -2252,7 +2274,8 @@ |
| 140 | |
| 141 | static int object_added_append_all(sd_bus *bus, sd_bus_message *m, const char *path) { |
| 142 | _cleanup_set_free_ Set *s = NULL; |
| 143 | - char *prefix; |
| 144 | + _cleanup_free_ char *prefix = NULL; |
| 145 | + size_t pl; |
| 146 | int r; |
| 147 | |
| 148 | assert(bus); |
| 149 | @@ -2297,7 +2320,12 @@ |
| 150 | if (bus->nodes_modified) |
| 151 | return 0; |
| 152 | |
| 153 | - prefix = alloca(strlen(path) + 1); |
| 154 | + pl = strlen(path); |
| 155 | + assert(pl <= BUS_PATH_SIZE_MAX); |
| 156 | + prefix = new(char, pl + 1); |
| 157 | + if (!prefix) |
| 158 | + return -ENOMEM; |
| 159 | + |
| 160 | OBJECT_PATH_FOREACH_PREFIX(prefix, path) { |
| 161 | r = object_added_append_all_prefix(bus, m, s, prefix, path, true); |
| 162 | if (r < 0) |
| 163 | @@ -2436,7 +2464,8 @@ |
| 164 | |
| 165 | static int object_removed_append_all(sd_bus *bus, sd_bus_message *m, const char *path) { |
| 166 | _cleanup_set_free_ Set *s = NULL; |
| 167 | - char *prefix; |
| 168 | + _cleanup_free_ char *prefix = NULL; |
| 169 | + size_t pl; |
| 170 | int r; |
| 171 | |
| 172 | assert(bus); |
| 173 | @@ -2468,7 +2497,12 @@ |
| 174 | if (bus->nodes_modified) |
| 175 | return 0; |
| 176 | |
| 177 | - prefix = alloca(strlen(path) + 1); |
| 178 | + pl = strlen(path); |
| 179 | + assert(pl <= BUS_PATH_SIZE_MAX); |
| 180 | + prefix = new(char, pl + 1); |
| 181 | + if (!prefix) |
| 182 | + return -ENOMEM; |
| 183 | + |
| 184 | OBJECT_PATH_FOREACH_PREFIX(prefix, path) { |
| 185 | r = object_removed_append_all_prefix(bus, m, s, prefix, path, true); |
| 186 | if (r < 0) |
| 187 | @@ -2618,7 +2652,8 @@ |
| 188 | const char *path, |
| 189 | const char *interface) { |
| 190 | |
| 191 | - char *prefix; |
| 192 | + _cleanup_free_ char *prefix = NULL; |
| 193 | + size_t pl; |
| 194 | int r; |
| 195 | |
| 196 | assert(bus); |
| 197 | @@ -2632,7 +2667,12 @@ |
| 198 | if (bus->nodes_modified) |
| 199 | return 0; |
| 200 | |
| 201 | - prefix = alloca(strlen(path) + 1); |
| 202 | + pl = strlen(path); |
| 203 | + assert(pl <= BUS_PATH_SIZE_MAX); |
| 204 | + prefix = new(char, pl + 1); |
| 205 | + if (!prefix) |
| 206 | + return -ENOMEM; |
| 207 | + |
| 208 | OBJECT_PATH_FOREACH_PREFIX(prefix, path) { |
| 209 | r = interfaces_added_append_one_prefix(bus, m, prefix, path, interface, true); |
| 210 | if (r != 0) |