Character limitation in config file, rest of line is read as new config line

[boolean.is.null]

There seems to be a character limitation, when the configuration file is read. The issue occurs after character number 255. The rest of the line is read as a valid line of configuration.

For instance when the following line is in the configuration:
tls-cipher TLS-DHE-RSA-WITH-AES-256-GCM-SHA384:TLS-DHE-RSA-WITH-AES-256-CBC-SHA256:TLS-DHE-RSA-WITH-AES-128-GCM-SHA256:TLS-DHE-RSA-WITH-AES-128-CBC-SHA256:TLS-DHE-RSA-WITH-CAMELLIA-256-CBC-SHA:TLS-DHE-RSA-WITH-AES-256-CBC-SHA:TLS-DHE-RSA-WITH-CAMELLIA-128-CBC-SHA

-CBC-SHA are read as a new line in the configuration. If this would be a valid option, it is configured successfully.

For instance:
tls-cipher TLS-DHE-RSA-WITH-AES-256-GCM-SHA384:TLS-DHE-RSA-WITH-AES-256-CBC-SHA256:TLS-DHE-RSA-WITH-AES-128-GCM-SHA256:TLS-DHE-RSA-WITH-AES-128-CBC-SHA256:TLS-DHE-RSA-WITH-CAMELLIA-256-CBC-SHA:TLS-DHE-RSA-WITH-AES-256-CBC-SHA:TLS-DHE-RSA-WITH-CAMELLIA-128push "dhcp-option DNS 8.8.8.8"

From character number 255 the option push "dhcp-option DNS 8.8.8.8" is evaluated as a valid option.

[Gert Döring]

Arne, can you have a quick look? I assume this is a static fgets() buffer overflowing (well, not "overflowing" but "splitting this into multiple lines") - not sure what the best fix would be, maybe just error out if the input line is too long?

@boolean.is.null: thanks for the report. You might want to reconsider whether you really want to specify all these :-)

[plaisthos]

We read OPTION_MAX_SIZE as a line. And when we read the next line the next line we do not check if there is anything left from the previous line.

[boolean.is.null]

Replying to cron2:

Arne, can you have a quick look? I assume this is a static fgets() buffer overflowing (well, not "overflowing" but "splitting this into multiple lines") - not sure what the best fix would be, maybe just error out if the input line is too long?

@boolean.is.null: thanks for the report. You might want to reconsider whether you really want to specify all these :-)

Thanks for the answer. Yes this seems like breeding ground to smuggle in various settings in config files, given by a service provider.
I'm aware that the list of tls-cipher is overkill, this is not actually used, it was for demonstration purposes.

I think an error message, something like config line length exceeded; line number; line is exceeded after [lasts 10 strings], would be appropriate

[boolean.is.null]

Replying to plaisthos:

We read OPTION_MAX_SIZE as a line. And when we read the next line the next line we do not check if there is anything left from the previous line.

If that is the expected behavior, then IMHO this should be explicitly printed. Either as a warning or a disclaimer.

[plaisthos]

I think we give an error.

In the push line that would degraded to a warning and we would still parse the rest of line. I don't think there is a seurity problem in the push line since the server does not get anymore possibilities to exploit the client.

[boolean.is.null]

Replying to plaisthos:

I think we give an error.

In the push line that would degraded to a warning and we would still parse the rest of line. I don't think there is a seurity problem in the push line since the server does not get anymore possibilities to exploit the client.

If I'm not mistaken, an error is given, if the rest of the line is not a valid line of configuration.
The possibility to hide a perhaps unwanted option, is what I see in this.

[Gert Döring]

commit 4baec3ee10b8d6826d5f076a9832a92a5cfe3676 (master)
commit 305c16c28d0ce7bc48c6e0abb18653241ddb910b (release/2.3)

Author: Arne Schwabe
Date: Thu Dec 10 13:37:10 2015 +0100

Detect config lines that are too long and give a warning/error

Acked-by: Gert Doering <gert@…>
Message-Id: <1449751030-10703-1-git-send-email-arne@…>
URL: ​http://article.gmane.org/gmane.network.openvpn.devel/10723

[ntj]

Replying to plaisthos:

We read OPTION_MAX_SIZE as a line. And when we read the next line the next line we do not check if there is anything left from the previous line.

Bump.
OPTION_MAX_SIZE is called OPTION_LINE_SIZE, and I'm not finding OPTION_MAX_SIZE in commit history, so this variable was may be called that way since it's been introduced.