# A bug in ipfw tool...



## cyberman (May 16, 2009)

Test rules:

```
-q set 12 flush
add set 12 allow tcp from any to me 12345 in
add set 12 allow tcp from me 12345 to any out
```
Result:

```
test# ipfw /root/fw_test.rule
Line 2: bad command `add'
```

The reason is that ipfw_main doesn't reset global variable 'use_set' before using it:

```
/*
	 * Optional: pipe, queue or nat.
	 */
	do_nat = 0;
	do_pipe = 0;
	if (!strncmp(*av, "nat", strlen(*av)))
 	        do_nat = 1;
 	else if (!strncmp(*av, "pipe", strlen(*av)))
		do_pipe = 1;
	else if (_substrcmp(*av, "queue") == 0)
		do_pipe = 2;
	else if (!strncmp(*av, "set", strlen(*av))) {
		if (ac > 1 && isdigit(av[1][0])) {
			use_set = strtonum(av[1], 0, RESVD_SET, &errstr);
			if (errstr)
				errx(EX_DATAERR,
				    "invalid set number %s\n", av[1]);
			ac -= 2; av += 2; use_set++;
		}
	}

	if (do_pipe || do_nat) {
		ac--;
		av++;
	}
	NEED1("missing command");

	/*
	 * For pipes, queues and nats we normally say 'nat|pipe NN config'
	 * but the code is easier to parse as 'nat|pipe config NN'
	 * so we swap the two arguments.
	 */
	if ((do_pipe || do_nat) && ac > 1 && isdigit(*av[0])) {
		char *p = av[0];

		av[0] = av[1];
		av[1] = p;
	}

	int try_next = 0;
	if (use_set == 0) {
		if (_substrcmp(*av, "add") == 0)
			add(ac, av);
		else if (do_nat && _substrcmp(*av, "show") == 0)
 			show_nat(ac, av);
		else if (do_pipe && _substrcmp(*av, "config") == 0)
			config_pipe(ac, av);
		else if (do_nat && _substrcmp(*av, "config") == 0)
 			config_nat(ac, av);
			else if (_substrcmp(*av, "set") == 0)
				sets_handler(ac, av);
			else if (_substrcmp(*av, "table") == 0)
				table_handler(ac, av);
			else if (_substrcmp(*av, "enable") == 0)
				sysctl_handler(ac, av, 1);
			else if (_substrcmp(*av, "disable") == 0)
				sysctl_handler(ac, av, 0);
			else
				try_next = 1;
	}

	if (use_set || try_next) {
		if (_substrcmp(*av, "delete") == 0)
			delete(ac, av);
		else if (_substrcmp(*av, "flush") == 0)
			flush(do_force);
		else if (_substrcmp(*av, "zero") == 0)
			zero(ac, av, IP_FW_ZERO);
		else if (_substrcmp(*av, "resetlog") == 0)
			zero(ac, av, IP_FW_RESETLOG);
		else if (_substrcmp(*av, "print") == 0 ||
		         _substrcmp(*av, "list") == 0)
			list(ac, av, do_acct);
		else if (_substrcmp(*av, "show") == 0)
			list(ac, av, 1 /* show counters */);
		else
			errx(EX_USAGE, "bad command `%s'", *av);
	}
```

I use 7.0R of FreeBSD, but 7.1 is the same. I haven't downloaded 7.2 yet, so I don't know if it's already fixed in the most recent release.


----------



## lme@ (May 18, 2009)

You can browse the repository online at:
http://cvsweb.frebsd.org or
http://svn.freebsd.org


----------



## cyberman (May 20, 2009)

lme@ said:
			
		

> You can browse the repository online at:
> http://cvsweb.frebsd.org or
> http://svn.freebsd.org



I've installed FreeBSD7.2 yestoday. The only change is that the newer version encapsulates these global variables into 'struct cmdline_opts' and defined a global struct variable 'co' of that type, but it never reset co.use_set while using rule file, I've tested it and the problem is still here.

From roadmap head is the current branch? I've read it, the same as 7.2.


----------



## lme@ (May 20, 2009)

If it is still not fixed in current, please file a PR for it.

TIA!


----------



## cyberman (May 26, 2009)

How to file a PR?


----------



## DutchDaemon (May 26, 2009)

http://www.freebsd.org/send-pr.html


----------

