Bug 7556 - ppp: sl_compress_init() will fail if called anything else than -1 or >MAX_STATE
Summary: ppp: sl_compress_init() will fail if called anything else than -1 or >MAX_STATE
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 2.2.6-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-net (Nobody)
URL:
Keywords: needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 1998-08-10 13:10 UTC by hm
Modified: 2020-03-08 13:12 UTC (History)
3 users (show)

See Also:
koobs: mfc-stable12?
koobs: mfc-stable11?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description hm 1998-08-10 13:10:00 UTC
The subroutine sl_compress_init(comp, max_state) in slcompress.c at least
implies, that the state table can be changed to any value dynamically if
called with some appropriate value for max_state.

This is not true, because the corresponding table is initialized by the
hardcoded MAX_STATES value in the header file slcompress.h.

Calling sl_compress_init() with a value greater than MAX_STATES will cause
cause writing outside the slcompress structure - bad things happen.

sl_compress_init() is currently used like this in the kernel PPP driver,
if_ppp.c:

#ifdef VJC
    case PPPIOCSMAXCID:
        if (error = suser(p->p_ucred, &p->p_acflag))
            return (error);
        s = splnet();
        sl_compress_init(&sc->sc_comp, *(int *)data);
        splx(s);
        break;
#endif

in this case, if this ioctl routine is ever issued with a value other
than -1 or with a value greater MAX_STATES, random writes into other
unknown data structures will occur!

Fix: 

Immediately disable the usage of any parameter for max_state other than
-1 now!

For the future, make the tstate and rstate structures in struct slcompress
resize dynamically so sl_compress_init() is able to do what it should be
able to do.
How-To-Repeat: 
Call sl_compress_init() with a max_state value of i.e. 64. (Caution:
make a backup before doing this!).
Comment 1 Brian Somers 1998-08-10 21:29:13 UTC
> 
> >Number:         7556
> >Category:       kern
> >Synopsis:       sl_compress_init() will fail if called anything else than -1 or >MAX_STATE
[.....]
If anyone picks this up (I haven't the time to be involved with 
pppd), there's an additional problem when a number of states is 
negotiated that != MAX_STATES.  Namely, it's possible that the peer 
may agree on (say) 8 states, then proceed to send a header with a 
slot id of (say) 10.  The end result is that a zero'd slot entry is 
``adjusted'' by the VJ deltas and will most likely cause a stack 
scribble.  We all know what happens to this in kernel mode :-/

This has been fixed in src/usr.sbin/ppp/slcompress.c - but I don't 
know how compatible the sources are.

-- 
Brian <brian@Awfulhak.org>, <brian@FreeBSD.org>, <brian@OpenBSD.org>
      <http://www.Awfulhak.org>
Don't _EVER_ lose your sense of humour....
Comment 2 Poul-Henning Kamp freebsd_committer 1998-08-14 07:10:44 UTC
State Changed
From-To: open->suspended

awaiting patch & committer 
Comment 3 Kris Kennaway freebsd_committer 2003-07-13 06:21:05 UTC
Responsible Changed
From-To: freebsd-bugs->hm

hm is a committer, let him decide whether his PR is still valid
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2004-09-13 06:35:51 UTC
Responsible Changed
From-To: hm->freebsd-bugs

Reassign to pool; maintainer is away from FreeBSD work due to press 
of other issues these days.
Comment 5 Eugene Grosbein freebsd_committer 2018-03-13 08:58:32 UTC
sl(4) was removed from FreeBSD before 8.0-RELEASE.
Comment 6 Alex Kozlov freebsd_committer 2018-03-13 10:18:04 UTC
This function is used in ng_vjc(4) and sppp(4). The issue described in this bug is still exist.