Bug #12315

Mitigate CVE-2017-2636 (LPE)

Added by cypherpunks 2017-03-09 10:05:17 . Updated 2017-04-19 17:25:06 .

Status:
Resolved
Priority:
High
Assignee:
Category:
Target version:
Start date:
2017-03-09
Due date:
% Done:

100%

Feature Branch:
bugfix/12315-CVE-2017-2636
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

Since Tails has CONFIG_N_HDLC=m in its kernel config, it is vulnerable to CVE-2017-2636. Calling TIOCSETD against a pseudoterminal will autoload the vulnerable module, allowing any user to escalate their privileges. See: https://security-tracker.debian.org/tracker/CVE-2017-2636

A mitigation is to blacklist n_hdlc in an /etc/modprobe.d/ entry.

Bug details:

>N_HDLC line discipline uses a self-made singly linked lists for data
>buffers and has n_hdlc.tbuf pointer for buffer retransmitting after
>an error. If sending of a data buffer is not successful, then its
>address is saved in n_hdlc.tbuf and the next time n_hdlc_send_frames()
>will try to resend it first of all.
>
>But the commit be10eb7589337e5defbe214dae038a53dd21add8 (“tty: n_hdlc add
>buffer flushing”) introduced racy access to n_hdlc.tbuf.
>
>After transmission error concurrent flush_tx_queue() and n_hdlc_send_frames()
>can put a buffer pointed by n_hdlc.tbuf to tx_free_buf_list twice. That
>causes an exploitable double free error in n_hdlc_release().
>
>To fix the issue I used a standard kernel linked list protected by a spinlock
>and got rid of n_hdlc.tbuf. In case of transmission error the current data
>buffer is put after the head of tx_buf_list.


Subtasks


History

#1 Updated by intrigeri 2017-03-09 10:14:07

  • Status changed from New to Confirmed
  • Assignee set to anonym
  • Priority changed from Urgent to High
  • Target version set to Tails_2.12

> A mitigation is to blacklist n_hdlc in an /etc/modprobe.d/ entry.

anonym, can you take care of this?

#2 Updated by cypherpunks 2017-03-11 10:11:16

This might be a useful way to what profiles are able to access the pseudoterminal. It’s not perfect because it only looks at one, and it doesn’t check for write permission for the character device. I don’t have a Tails system handy right now, but manual analysis makes it look like Tor Browser cannot access the pseudoterminals.

cat /sys/kernel/security/apparmor/policy/profile/*/name 2>/dev/null | while read -r i; do
    if aa-exec -p "$i" -- stty -a < /dev/pts/0 >/dev/null; then
        echo "$i"
    fi
done

Is resolving this bug going to take until the next version of Tails, or is an emergency release going to be issued for this quite severe vulnerability?

#3 Updated by intrigeri 2017-03-19 09:17:38

I want it to be fixed in 3.0~beta3 (today).

#4 Updated by intrigeri 2017-03-19 10:18:00

  • Assignee changed from anonym to intrigeri

… so I’ll take it.

#5 Updated by intrigeri 2017-03-19 10:20:36

cypherpunks wrote:
> Is resolving this bug going to take until the next version of Tails,

Yes: 2.12 and 3.0~beta3.

> or is an emergency release going to be issued for this quite severe vulnerability?

We can’t afford doing emergency releases for every local priv esc :/ … yet. (Reproducible builds will make this a bit cheaper.)

#6 Updated by intrigeri 2017-03-19 11:13:51

  • Status changed from Confirmed to In Progress
  • Assignee changed from intrigeri to anonym
  • % Done changed from 0 to 50
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/12315-CVE-2017-2636

#7 Updated by cypherpunks 2017-03-30 02:39:49

intrigeri wrote:
> cypherpunks wrote:
> > or is an emergency release going to be issued for this quite severe vulnerability?
> We can’t afford doing emergency releases for every local priv esc :/ … yet. (Reproducible builds will make this a bit cheaper.)
I hope that at least the upgrade importance will be rated as high, instead of medium as it is every other time.

#8 Updated by intrigeri 2017-03-30 08:58:59

> I hope that at least the upgrade importance will be rated as high, instead of medium as it is every other time.

What’s the “upgrade importance”?

#9 Updated by cypherpunks 2017-04-01 03:19:47

intrigeri wrote:
> > I hope that at least the upgrade importance will be rated as high, instead of medium as it is every other time.
>
> What’s the “upgrade importance”?

Urgency, I mean. For example, from the changelogs:

tails (3.0~beta3) unstable; urgency=medium

#10 Updated by intrigeri 2017-04-01 06:23:30

> Urgency, I mean. For example, from the changelogs:

>

> tails (3.0~beta3) unstable; urgency=medium
> 

I see. Thankfully we don’t communicate this to users as “upgrade importance”.

#11 Updated by cypherpunks 2017-04-03 01:56:40

intrigeri wrote:
> > Urgency, I mean. For example, from the changelogs:
>
> > […]
>
> I see. Thankfully we don’t communicate this to users as “upgrade importance”.
This has confused me and others I have talked to in the past, then, considering users are linked to the changelog. Perhaps it should be clarified or removed in that case, but that’s for another ticket.

#12 Updated by anonym 2017-04-03 18:13:52

  • Status changed from In Progress to Fix committed
  • Assignee deleted (anonym)
  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

#13 Updated by anonym 2017-04-19 17:25:06

  • Status changed from Fix committed to Resolved