ip xfrm state add crashes when supplied an algo

Bug #1220782 reported by ender wiggin
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
iproute2 (Ubuntu)
Expired
Undecided
Unassigned

Bug Description

This is valid for iproute2-ss111117 which is default on Ubuntu 12.04 (LTS).

The crash appends when supplying enc, auth, comp, auth-trunc or aead and the following key argumentis given as a string (as opposed to hexadecimal). When trying to copy the key, it generates a segfault:
------------------------------------------------------------
*** buffer overflow detected ***: /sbin/ip terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7ffff7921807]
/lib/x86_64-linux-gnu/libc.so.6(+0x109700)[0x7ffff7920700]
/lib/x86_64-linux-gnu/libc.so.6(+0x1089e6)[0x7ffff791f9e6]
/sbin/ip[0x420d84]
/sbin/ip(do_xfrm_state+0x7a1)[0x421951]
/sbin/ip[0x405ad5]
/sbin/ip(main+0x2b4)[0x4056d4]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7ffff783876d]
/sbin/ip[0x405959]
------------------------------------------------------------

This buffer overflow is actually preposterous since __strncpy_chk is called with 0 as its last argument:
   0x0000000000420d72: mov 0x68(%rsp),%rsi
   0x0000000000420d77: movslq %r12d,%rdx
   0x0000000000420d7a: xor %ecx,%ecx
   0x0000000000420d7c: mov %r9,%rdi
=> 0x0000000000420d7f: callq 0x404a10 <__strncpy_chk@plt>

("xor %ecx,%ecx" set the fourth argument to 0)

Which is equivalent to:
__strncpy_chk(buf, key, len, 0);

When obtaining the source package, the corresponding code looks like that (from ip/xfrm_state.c, line 113):

static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
                           char *name, char *key, char *buf, int max)
{
        int len;
        int slen = strlen(key);

#if 0
        /* XXX: verifying both name and key is required! */
        fprintf(stderr, "warning: ALGO-NAME/ALGO-KEY will send to kernel promiscuously! (verifying them isn't implemented yet)\n");
#endif

        strncpy(alg->alg_name, name, sizeof(alg->alg_name));

        if (slen > 2 && strncmp(key, "0x", 2) == 0) {
<snip>
        } else {
                len = slen;
                if (len > 0) {
                        if (len > max)
                                invarg("\"ALGO-KEY\" makes buffer overflow\n", key);

                        strncpy(buf, key, len); <----- correct line which is faulty in the assembly
                }
        }

        alg->alg_key_len = len * 8;

        return 0;
}

This code is actually valid but doesn't match the assembly from the binary package.

This is quite annoying since it prevents from setting keys in a string form, thus forcing the user to derivates itself the hexadecimal value for the key when trying to setup an IPsec tunnel for example.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
I come by cleaning up old issues.
So first of all I beg your pardon that nobody seems to ever have looked at that.
Never the less I have to ask you if this would still happen on newer still supported releases?

If so, it would be great if you could add the steps to reproduce.
Essentially convert the sentence "when supplying enc, auth, comp, auth-trunc or aead and the following key argumentis given as a string" into a series of commands to ensure one does the same when trying to reproduce.

Changed in iproute2 (Ubuntu):
status: New → Incomplete
Revision history for this message
Launchpad Janitor (janitor) wrote :

[Expired for iproute2 (Ubuntu) because there has been no activity for 60 days.]

Changed in iproute2 (Ubuntu):
status: Incomplete → Expired
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.