PF ioctl(DIOCADDADDR) possible bug

[Daniel Hartmeier]

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;