On Sunday 2009-11-01 11:53, Pierre Chifflier wrote:
>This input plugins creates a unix socket which can be used to log packets.
>Scripts or applications can connect to the socket (only one client allowed
>per socket) and send data in a Key-Length-Value format (including the
>payload).
>
>+#define UNIX_PATH_MAX 108
This is nowhere used and can be removed. (Also since 108 may be
wrong for a given arch.)
>+#define unixpath_ce(x) (x->ces[0])
>+#define bufsize_ce(x) (x->ces[1])
>+#define perms_ce(x) (x->ces[2])
>+#define owner_ce(x) (x->ces[3])
>+#define group_ce(x) (x->ces[4])
x should be guarded with ((x)->ces[0])
>+static int handle_packet(struct ulogd_pluginstance *upi, char *packet_buf, u_int16_t total_len)
>+{
>+ char *data;
>+ struct iphdr *ip;
>+ struct ulogd_key *ret = upi->output.keys;
>+ u_int8_t oob_family;
>+ u_int16_t payload_len;
>+ u_int16_t option_number;
>+ u_int16_t option_length;
>+ char *buf;
>+
>+ ulogd_log(ULOGD_DEBUG,
>+ "ulogd2: handling packet\n");
>+
>+ data = packet_buf + sizeof(u_int16_t);
>+
>+ payload_len = ntohs(*(u_int16_t*)data);
>+ data += sizeof(u_int16_t);
>+
>+
>+ ip = (struct iphdr *) data;
If data is not aligned, later dereferencing can fail :(
>+ if (ip->version == 4)
>+ oob_family = AF_INET;
>+ else if (ip->version == 6)
>+ oob_family = AF_INET6;
>+ else oob_family = 0;
>+
>+ okey_set_u8(&ret[UNIXSOCK_KEY_OOB_FAMILY], oob_family);
>+
>+ okey_set_ptr(&ret[UNIXSOCK_KEY_RAW_PCKT], ip);
>+ okey_set_u32(&ret[UNIXSOCK_KEY_RAW_PCKTLEN], payload_len);
>+
>+ /* options */
>+ if (total_len > payload_len + sizeof(u_int16_t)) {
>+ data = packet_buf + payload_len + 2*sizeof(u_int16_t);
>+ total_len -= 2*sizeof(u_int16_t);
>+
>+ while ( (data - packet_buf) < total_len) {
>+
>+ option_number = ntohs(*(u_int16_t*)(data));
This can fail even if packet_buf _is_ aligned, all it takes
is that payload_len is an odd number.
Unfortunately, I see no way to easily address this without
lame bitshifting :(
>+static int _unix_socket_set_permissions(struct ulogd_pluginstance *upi)
>+{
>+ const char *socket_path;
>+ char *owner = owner_ce(upi->config_kset).u.string;
>+ char *group = group_ce(upi->config_kset).u.string;
const char *, I guess
>+ int uid = -1;
>+ int gid = -1;
uit_t, gid_t.
>+ if (uid >= 0 || gid >= 0) {
Subsequently, uid != -1 || gid != -1. However, you could also ignore
this check entirely since chown checks for -1 already.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
