Bug 202540 - libteken assert() fail on teken.c line 231
Summary: libteken assert() fail on teken.c line 231
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: FreeBSD bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-21 05:20 UTC by kcwu
Modified: 2015-09-14 09:31 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kcwu 2015-08-21 05:20:36 UTC
Because syscons (a kernel driver) uses libteken, an assertion failure in libteken would result in kernel panic.

How to reproduce:
To reproduce the assertion reliably, I modified src/sys/teken/stress/teken_stress.c to take input from stdin.
$ echo -e '\e[?6h\e[5r\e[65533H' | ./teken_stress 
Assertion failed: (t->t_cursor.tp_row >= t->t_originreg.ts_begin), function teken_input_char, file /usr/src/sys/teken/teken.c, line 231.
Abort trap (core dumped)

This issue is found by afl-fuzz
This is similar but different case to https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202326 . Please let me know if you want me report in the same tracking bug.
Comment 1 commit-hook freebsd_committer 2015-08-21 06:30:49 UTC
A commit references this bug:

Author: ed
Date: Fri Aug 21 06:30:13 UTC 2015
New revision: 286981
URL: https://svnweb.freebsd.org/changeset/base/286981

Log:
  Don't truncate cursor arithmetic to 16 bits.

  When updating the row number when the cursor position escape sequence is
  issued, we should make sure to store the intermediate result in a 32-bit
  integer. If we fail to do this, the cursor may be set above the origin
  region, which is bad.

  This could cause libteken to crash when INVARIANTS is enabled, due to
  the strict set of assertions that libteken has.

  PR:		202540
  Reported by:	kcwu csie org
  MFC after:	1 month

Changes:
  head/sys/teken/teken_subr.h
Comment 2 Ed Schouten freebsd_committer 2015-08-21 06:32:21 UTC
Thanks again! Really appreciated!

Be sure to file separate bug reports. A single bug report would easily get cluttered and would make it easy for me to forget to MFC the change.
Comment 3 kcwu 2015-08-21 07:06:11 UTC
BTW, do you mean libteken's assertion is only enabled when kernel compiled with INVARIANTS ? Then these two bugs are more serious from security aspect for -stable than I thought.
Comment 4 Ed Schouten freebsd_committer 2015-08-21 07:25:00 UTC
Exactly. In stable these are not enabled.

Though I agree that these issues are pretty bad, we usually treat the console as something that only trusted people can access. As far as I know, there isn't a way to print arbitrary, unescaped data on one of vt(4)'s windows without having a local login session.

Earlier this year we wrote an erratum for something similar: a bad ioctl() call on vt(4) that could cause the system to crash.

https://www.freebsd.org/security/advisories/FreeBSD-EN-15:01.vt.asc
Comment 5 commit-hook freebsd_committer 2015-09-14 09:13:10 UTC
A commit references this bug:

Author: ed
Date: Mon Sep 14 09:12:30 UTC 2015
New revision: 287776
URL: https://svnweb.freebsd.org/changeset/base/287776

Log:
  MFC r286798 and r286827:

    Stop parsing digits if the value already exceeds UINT_MAX / 100.

    There is no need for us to support parsing values that are larger than
    the maximum terminal window size. In this case that would be the maximum
    of unsigned short.

    The problem with parsing larger values is that they can cause integer
    overflows when adjusting the cursor position, leading to all sorts of
    failing assertions.

  MFC r286981 and r287098:

    Don't truncate cursor arithmetic to 16 bits.

    When updating the row number when the cursor position escape sequence is
    issued, we should make sure to store the intermediate result in a 32-bit
    integer. If we fail to do this, the cursor may be set above the origin
    region, which is bad.

    This could cause libteken to crash when INVARIANTS is enabled, due to
    the strict set of assertions that libteken has.

  PR:		202326, 202540, 202612
  Submitted by:	kwcu csie org

Changes:
_U  stable/10/
  stable/10/sys/teken/teken.c
  stable/10/sys/teken/teken_subr.h
Comment 6 commit-hook freebsd_committer 2015-09-14 09:31:16 UTC
A commit references this bug:

Author: ed
Date: Mon Sep 14 09:31:01 UTC 2015
New revision: 287777
URL: https://svnweb.freebsd.org/changeset/base/287777

Log:
  MFC r286798 and r286827:

    Stop parsing digits if the value already exceeds UINT_MAX / 100.

    There is no need for us to support parsing values that are larger than
    the maximum terminal window size. In this case that would be the maximum
    of unsigned short.

    The problem with parsing larger values is that they can cause integer
    overflows when adjusting the cursor position, leading to all sorts of
    failing assertions.

  MFC r286981 and r287098:

    Don't truncate cursor arithmetic to 16 bits.

    When updating the row number when the cursor position escape sequence is
    issued, we should make sure to store the intermediate result in a 32-bit
    integer. If we fail to do this, the cursor may be set above the origin
    region, which is bad.

    This could cause libteken to crash when INVARIANTS is enabled, due to
    the strict set of assertions that libteken has.

  PR:		202326, 202540, 202612
  Submitted by:	kwcu csie org

Changes:
_U  stable/9/
_U  stable/9/sys/
  stable/9/sys/teken/teken.c
  stable/9/sys/teken/teken_subr.h