netdev - tp->copied_seq used before assignment in tcp_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHx7fy4nNq-iWVGF7CWuDi8W_BDRVLQg3QfS_R54eEO5bsXj3Q@mail.gmail.com>
Date:   Sun, 9 Jun 2019 19:59:17 -0700
From:   Zhongjie Wang <zwang048@....edu>
To:     netdev@...r.kernel.org, Zhiyun Qian <zhiyunq@...ucr.edu>
Subject: tp->copied_seq used before assignment in tcp_check_urg

Hi all,

We are a group of researchers at UC Riverside. We recently discovered
some weird case
in the TCP code in which the tp->copied_seq variable is used before
assignment to an
appropriate TCP sequence number. It's discovered with our symbolic
execution tool,
so we'd like to seek confirmation of the correctness of the discovery.

In tcp_input.c, tcp_check_urg() function:

if (tp->urg_seq == tp->copied_seq && tp->urg_data &&
    !sock_flag(sk, SOCK_URGINLINE) && tp->copied_seq != tp->rcv_nxt) {
        struct sk_buff *skb = skb_peek(&sk->sk_receive_queue);
        tp->copied_seq++;
        if (skb && !before(tp->copied_seq, TCP_SKB_CB(skb)->end_seq)) {
                __skb_unlink(skb, &sk->sk_receive_queue);
                __kfree_skb(skb);
        }
}

It compares tp->copied_seq with tcp->rcv_nxt.
However, tp->copied_seq is only assigned to an appropriate sequence number when
it copies data to user space. So here tp->copied_seq could be equal to 0,
which is its initial value, if no data are copied yet.
In this case, the condition becomes 0 != tp->rcv_nxt,
and it renders this comparison ineffective.

For example, if we send a SYN packet with initial sequence number 0xFF FF FF FF,
and after receiving SYN/ACK response, then send a ACK packet with sequence
number 0, it will bypass this if-then block.

We are not sure how this would affect the TCP logic. Could you please confirm
that tp->copied_seq should be assigned to a sequence number before its use?

Best,
Zhongjie Wang
Ph.D. Candidate 2015 Fall
Department of Computer Science & Engineering
University of California, Riverside