Commit 74c8d4d1 authored by ballock's avatar ballock

[firewall] Fix iptables validator bugs. JB#48869

clean_match_option
------------------

This example rule would fail, while being correct:
-p tcp -m tcp ! --tcp-option 1 -m tcp ! --tcp-option 2 -j ACCEPT

The reason is that tcp match has AF_UNSPEC instead of the particular
AF_INET/AF_INET6 protocol specified in its requirements.

Although it's possible to add a more complicated check (first to see,
if the match's family_dep is not UNSPEC, then if it equals the family,
but there's no need to since we only execute cleanup when the match
was already in the invoked_matches list, and it could have been there
only if it meets the conditions of the match.

Ergo: this check is unnecessary and harmful.

is_valid_port_or_service_range
------------------------------

tokens[i] will only be false at the end of the vector, not when the
string is empty. Thus, this check does what the g_strv_length does.

handle_ports
------------

It turned out that the function allows to use an empty
string ('') as a correct value, which should not be allowed.

Since like with the previous function, we can either use tokens[i] to
indicate vector end or g_strv_length. However, since we need to verify
that the split vector is empty, this form was chosen.
parent 872ab71c
......@@ -286,7 +286,7 @@ static bool is_valid_port_or_service_range(const char *protocol,
/* Range can have only two set */
if (g_strv_length(tokens) == 2) {
for (i = 0; i < 2 && tokens[i]; i++) {
for (i = 0; i < 2; i++) {
if (!is_valid_port_or_service(protocol, tokens[i],
&ports[i])) {
DBG("invalid port/service %s in %s", tokens[i],
......@@ -659,6 +659,7 @@ static bool handle_ports(struct validator_data *data, gchar **args)
struct protoent *p;
char *protoname = NULL;
gchar **tokens = NULL;
int token_count;
/* In iptables ports are separated with commas, ranges with colon. */
const char delimeter[] = ",";
......@@ -674,7 +675,11 @@ static bool handle_ports(struct validator_data *data, gchar **args)
if (!tokens)
return false;
for (i = 0; tokens[i]; i++) {
token_count = g_strv_length(tokens);
if (token_count < 1)
ret = false;
for (i = 0; i < token_count; i++) {
/*
* If ':' exists it is a range. Check that only one ':' exists
* and the port range is specified correctly
......@@ -1571,9 +1576,7 @@ static void clean_match_options(struct validator_data *data, gchar *match)
int i;
for (i = 0; known_matches[i].match_name; i++) {
if (!g_strcmp0(known_matches[i].match_name, match) &&
known_matches[i].family_dep == data->family) {
if (!g_strcmp0(known_matches[i].match_name, match)) {
remove_unique_ids_from_invoked(data,
known_matches[i].opts_enabled);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment