From owner-freebsd-pf@FreeBSD.ORG Fri Aug 5 11:34:15 2005 Return-Path: X-Original-To: freebsd-pf@freebsd.org Delivered-To: freebsd-pf@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E740316A41F for ; Fri, 5 Aug 2005 11:34:15 +0000 (GMT) (envelope-from dhartmei@insomnia.benzedrine.cx) Received: from insomnia.benzedrine.cx (insomnia.benzedrine.cx [62.65.145.30]) by mx1.FreeBSD.org (Postfix) with ESMTP id 27C9043D53 for ; Fri, 5 Aug 2005 11:34:14 +0000 (GMT) (envelope-from dhartmei@insomnia.benzedrine.cx) Received: from insomnia.benzedrine.cx (dhartmei@localhost [127.0.0.1]) by insomnia.benzedrine.cx (8.13.4/8.12.11) with ESMTP id j75BYEE4031038 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Fri, 5 Aug 2005 13:34:14 +0200 (MEST) Received: (from dhartmei@localhost) by insomnia.benzedrine.cx (8.13.4/8.12.10/Submit) id j75BYEHP000272; Fri, 5 Aug 2005 13:34:14 +0200 (MEST) Date: Fri, 5 Aug 2005 13:34:14 +0200 From: Daniel Hartmeier To: Boris Polevoy Message-ID: <20050805113413.GR11104@insomnia.benzedrine.cx> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.6i Cc: pf@benzedrine.cx, freebsd-pf@freebsd.org Subject: Re: PF ioctl(DIOCADDADDR) possible bug X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Technical discussion and general questions about packet filter \(pf\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Aug 2005 11:34:16 -0000 On Fri, Aug 05, 2005 at 03:06:19PM +0400, Boris Polevoy wrote: > In step 2 ioctl(DIOCADDADDR) do not check pool ticket value, and there is possible situation of malicious or failure > address pool addition whithout geting pool ticket from another process. > > Is it bug or not? Yes, I think it's an oversight to not check the ticket in DIOCADDADDR. Depending on timing, one of two concurrent processes could add additional addresses into the temporary pool that the other process will then commit. The first one will get an error when trying to commit. There won't be any data corruption or crashes or such, just the first process has inserted one or more addresses into the pool that the second process is commiting. This is more of an issue when it happens by accident. A malicious process with privileges to /dev/pf could produce the same (and worse) results more easily without relying on this missing check, of course. With the patch below (applies to both OpenBSD -current and FreeBSD RELENG_5), this is prevented. Daniel Index: pf_ioctl.c =================================================================== RCS file: /cvs/src/sys/net/pf_ioctl.c,v retrieving revision 1.152 diff -u -r1.152 pf_ioctl.c --- pf_ioctl.c 5 Aug 2005 09:03:19 -0000 1.152 +++ pf_ioctl.c 5 Aug 2005 11:21:40 -0000 @@ -2195,6 +2195,10 @@ case DIOCADDADDR: { struct pfioc_pooladdr *pp = (struct pfioc_pooladdr *)addr; + if(pp->ticket != ticket_pabuf) { + error = EBUSY; + break; + } #ifndef INET if (pp->af == AF_INET) { error = EAFNOSUPPORT;