2 files changed, 128 insertions, 80 deletions

diff --git a/sbin/ipfw/ipfw.8 b/sbin/ipfw/ipfw.8
index 2011dd2b0830..9183b2a4569d 100644
--- a/sbin/ipfw/ipfw.8
+++ b/sbin/ipfw/ipfw.8
@@ -1,7 +1,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd May 24, 2006
+.Dd June 15, 2006
 .Dt IPFW 8
 .Os
 .Sh NAME
@@ -1587,12 +1587,16 @@ Lookup tables currently support IPv4 addresses only.
 The
 .Cm tablearg
 feature provides the ability to use a value, looked up in the table, as
-the argument for a rule action.
+the argument for a rule action, action parameter or rule option.
 This can significantly reduce number of rules in some configurations.
 The
 .Cm tablearg
 argument can be used with the following actions:
-.Cm pipe , queue, divert, tee, netgraph, ngtee .
+.Cm pipe , queue, divert, tee, netgraph, ngtee,
+action parameters:
+.Cm tag, untag,
+rule options:
+.Cm limit, tagged.
 See the
 .Sx EXAMPLES
 Section for example usage of tables and the tablearg keyword.
diff --git a/sbin/ipfw/ipfw2.c b/sbin/ipfw/ipfw2.c
index ddaf4ac8ae5e..682a52563b2b 100644
--- a/sbin/ipfw/ipfw2.c
+++ b/sbin/ipfw/ipfw2.c
@@ -82,13 +82,42 @@ int
  */
 #define NEED1(msg)      {if (!ac) errx(EX_USAGE, msg);}
 
-#define NOT_NUMBER(str, msg) do {		\
-	char *c;				\
-	for (c = str; *c != '\0'; c++) {	\
-		if (isdigit(*c))		\
-			continue;		\
-		errx(EX_DATAERR, msg);		\
-	}					\
+#define GET_UINT_ARG(arg, min, max, tok, s_x) do {			\
+	if (!ac)							\
+		errx(EX_USAGE, "%s: missing argument", match_value(s_x, tok)); \
+	if (_substrcmp(*av, "tablearg") == 0) {				\
+		arg = IP_FW_TABLEARG;					\
+		break;							\
+	}								\
+									\
+	{								\
+	long val;							\
+	char *end;							\
+									\
+	val = strtol(*av, &end, 10);					\
+									\
+	if (!isdigit(**av) || *end != '\0' || (val == 0 && errno == EINVAL)) \
+		errx(EX_DATAERR, "%s: invalid argument: %s",		\
+		    match_value(s_x, tok), *av);			\
+									\
+	if (errno == ERANGE || val < min || val > max)			\
+		errx(EX_DATAERR, "%s: argument is out of range (%u..%u): %s", \
+		    match_value(s_x, tok), min, max, *av);		\
+									\
+	if (val == IP_FW_TABLEARG)					\
+		errx(EX_DATAERR, "%s: illegal argument value: %s",	\
+		    match_value(s_x, tok), *av);			\
+	arg = val;							\
+	}								\
+} while (0)
+
+#define PRINT_UINT_ARG(str, arg) do {					\
+	if (str != NULL)						\
+		printf("%s",str);					\
+	if (arg == IP_FW_TABLEARG)					\
+		printf("tablearg");					\
+	else								\
+		printf("%u", (uint32_t)arg);				\
 } while (0)
 
 /*
@@ -801,30 +830,39 @@ fill_newports(ipfw_insn_u16 *cmd, char *av, int proto)
 
 	while (*s) {
 		a = strtoport(av, &s, 0, proto);
-		if (s == av) /* no parameter */
-			break;
-		if (*s == '-') { /* a range */
-			av = s+1;
+		if (s == av) 			/* empty or invalid argument */
+			return (0);
+
+		switch (*s) {
+		case '-':			/* a range */
+			av = s + 1;
 			b = strtoport(av, &s, 0, proto);
-			if (s == av) /* no parameter */
-				break;
+			/* Reject expressions like '1-abc' or '1-2-3'. */
+			if (s == av || (*s != ',' && *s != '\0'))
+				return (0);
 			p[0] = a;
 			p[1] = b;
-		} else if (*s == ',' || *s == '\0' )
+			break;
+		case ',':			/* comma separated list */
+		case '\0':
 			p[0] = p[1] = a;
-		else 	/* invalid separator */
-			errx(EX_DATAERR, "invalid separator <%c> in <%s>\n",
+			break;
+		default:
+			warnx("port list: invalid separator <%c> in <%s>",
 				*s, av);
+			return (0);
+		}
+
 		i++;
 		p += 2;
-		av = s+1;
+		av = s + 1;
 	}
 	if (i > 0) {
-		if (i+1 > F_LEN_MASK)
+		if (i + 1 > F_LEN_MASK)
 			errx(EX_DATAERR, "too many ports/ranges\n");
-		cmd->o.len |= i+1; /* leave F_NOT and F_OR untouched */
+		cmd->o.len |= i + 1;	/* leave F_NOT and F_OR untouched */
 	}
-	return i;
+	return (i);
 }
 
 static struct _s_x icmpcodes[] = {
@@ -1475,28 +1513,33 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 				print_unreach6_code(cmd->arg1);
 			break;
 
-#define	PRINT_WITH_ARG(o)						\
-			if (cmd->arg1 == IP_FW_TABLEARG)		\
-				printf("%s tablearg", (o));		\
-			else						\
-				printf("%s %u", (o), cmd->arg1);	\
+		case O_SKIPTO:
+			PRINT_UINT_ARG("skipto ", cmd->arg1);
 			break;
 
-		case O_SKIPTO:
-			PRINT_WITH_ARG("skipto");
 		case O_PIPE:
-			PRINT_WITH_ARG("pipe");
+			PRINT_UINT_ARG("pipe ", cmd->arg1);
+			break;
+
 		case O_QUEUE:
-			PRINT_WITH_ARG("queue");
+			PRINT_UINT_ARG("queue ", cmd->arg1);
+			break;
+
 		case O_DIVERT:
-			PRINT_WITH_ARG("divert");
+			PRINT_UINT_ARG("divert ", cmd->arg1);
+			break;
+
 		case O_TEE:
-			PRINT_WITH_ARG("tee");
+			PRINT_UINT_ARG("tee ", cmd->arg1);
+			break;
+
 		case O_NETGRAPH:
-			PRINT_WITH_ARG("netgraph");
+			PRINT_UINT_ARG("netgraph ", cmd->arg1);
+			break;
+
 		case O_NGTEE:
-			PRINT_WITH_ARG("ngtee");
-#undef PRINT_WITH_ARG
+			PRINT_UINT_ARG("ngtee ", cmd->arg1);
+			break;
 
 		case O_FORWARD_IP:
 		    {
@@ -1542,9 +1585,9 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 	}
 	if (tagptr) {
 		if (tagptr->len & F_NOT)
-			printf(" untag %hu", tagptr->arg1);
+			PRINT_UINT_ARG(" untag ", tagptr->arg1);
 		else
-			printf(" tag %hu", tagptr->arg1);
+			PRINT_UINT_ARG(" tag ", tagptr->arg1);
 	}
 
 	/*
@@ -1898,8 +1941,7 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 				printf(" keep-state");
 				break;
 
-			case O_LIMIT:
-			    {
+			case O_LIMIT: {
 				struct _s_x *p = limit_masks;
 				ipfw_insn_limit *c = (ipfw_insn_limit *)cmd;
 				uint8_t x = c->limit_mask;
@@ -1912,9 +1954,9 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 						printf("%s%s", comma, p->s);
 						comma = ",";
 					}
-				printf(" %d", c->conn_limit);
-			    }
+				PRINT_UINT_ARG(" ", c->conn_limit);
 				break;
+			}
 
 			case O_IP6:
 				printf(" ip6");
@@ -1934,10 +1976,10 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 
 			case O_TAGGED:
 				if (F_LEN(cmd) == 1)
-				    printf(" tagged %hu", cmd->arg1 );
+					PRINT_UINT_ARG(" tagged ", cmd->arg1);
 				else
-				    print_newports((ipfw_insn_u16 *)cmd, 0,
-					O_TAGGED);
+					print_newports((ipfw_insn_u16 *)cmd, 0,
+					    O_TAGGED);
 				break;
 
 			default:
@@ -4061,18 +4103,18 @@ chkarg:
 			break;
 
 		case TOK_TAG:
-		case TOK_UNTAG:
-			{
+		case TOK_UNTAG: {
+			uint16_t tag;
+
 			if (have_tag)
-				errx(EX_USAGE, "tag and untag cannot be specified more than once");
-			NEED1("missing tag number");
-			NOT_NUMBER(*av, "invalid tag number");
+				errx(EX_USAGE, "tag and untag cannot be "
+				    "specified more than once");
+			GET_UINT_ARG(tag, 1, 65534, i, rule_action_params);
 			have_tag = cmd;
-			fill_cmd(cmd, O_TAG, (i == TOK_TAG) ? 0: F_NOT,
-			    strtoul(*av, NULL, 0));
+			fill_cmd(cmd, O_TAG, (i == TOK_TAG) ? 0: F_NOT, tag);
 			ac--; av++;
-			}
 			break;
+		}
 
 		default:
 			abort();
@@ -4517,39 +4559,38 @@ read_options:
 			fill_cmd(cmd, O_KEEP_STATE, 0, 0);
 			break;
 
-		case TOK_LIMIT:
+		case TOK_LIMIT: {
+			ipfw_insn_limit *c = (ipfw_insn_limit *)cmd;
+			int val;
+
 			if (open_par)
-				errx(EX_USAGE, "limit cannot be part "
-				    "of an or block");
+				errx(EX_USAGE,
+				    "limit cannot be part of an or block");
 			if (have_state)
-				errx(EX_USAGE, "only one of keep-state "
-					"and limit is allowed");
-			NEED1("limit needs mask and # of connections");
+				errx(EX_USAGE, "only one of keep-state and"
+				    "limit is allowed");
 			have_state = cmd;
-		    {
-			ipfw_insn_limit *c = (ipfw_insn_limit *)cmd;
 
 			cmd->len = F_INSN_SIZE(ipfw_insn_limit);
 			cmd->opcode = O_LIMIT;
-			c->limit_mask = 0;
-			c->conn_limit = 0;
-			for (; ac >1 ;) {
-				int val;
+			c->limit_mask = c->conn_limit = 0;
 
-				val = match_token(limit_masks, *av);
-				if (val <= 0)
+			while (ac > 0) {
+				if ((val = match_token(limit_masks, *av)) <= 0)
 					break;
 				c->limit_mask |= val;
 				ac--; av++;
 			}
-			c->conn_limit = atoi(*av);
-			if (c->conn_limit == 0)
-				errx(EX_USAGE, "limit: limit must be >0");
+
 			if (c->limit_mask == 0)
-				errx(EX_USAGE, "missing limit mask");
+				errx(EX_USAGE, "limit: missing limit mask");
+
+			GET_UINT_ARG(c->conn_limit, 1, 65534, TOK_LIMIT,
+			    rule_options);
+
 			ac--; av++;
-		    }
 			break;
+		}
 
 		case TOK_PROTO:
 			NEED1("missing protocol");
@@ -4664,14 +4705,17 @@ read_options:
 			break;
 
 		case TOK_TAGGED:
-			NEED1("missing tag number");
-			if (strpbrk(*av, "-,")) {
+			if (ac > 0 && strpbrk(*av, "-,")) {
 				if (!add_ports(cmd, *av, 0, O_TAGGED))
-					errx(EX_DATAERR, "invalid tag %s", *av);
-			} else {
-				NOT_NUMBER(*av, "invalid tag number");
-				fill_cmd(cmd, O_TAGGED, 0,
-				    strtoul(*av, NULL, 0));
+					errx(EX_DATAERR, "tagged: invalid tag"
+					    " list: %s", *av);
+			}
+			else {
+				uint16_t tag;
+
+				GET_UINT_ARG(tag, 1, 65534, TOK_TAGGED,
+				    rule_options);
+				fill_cmd(cmd, O_TAGGED, 0, tag);
 			}
 			ac--; av++;
 			break;