#528095 network-manager: puts severe restrictions on the formatting of /etc/network/interfaces

Package:
network-manager
Source:
network-manager
Description:
network management framework (daemon and userspace tools)
Submitter:
Jö Fahlke
Date:
2024-06-24 10:51:02 UTC
Severity:
important
#528095#5
Date:
2009-05-10 19:27:24 UTC
From:
To:
There are several problems in the function ifparser_init() from
system-settings/plugins/ifupdown/interface_parser, which I assume is used to
parse /etc/network/interfaces:

 * Indentation or word seperation using tabs is unsupported.

 * Word seperation by more than one space is unsupported.

 * Line continuation using '\\\n' is unsupported.

 * Lines of length 255 or longer (not including '\n') cause an off-by-one
   error.  Indeed, inserting a line of 255 times '#' make nm-system-settings
   segfault:

   | May 10 20:00:48 paranoia NetworkManager: <info>  Trying to start the system settings daemon...
   | May 10 20:00:48 paranoia nm-system-settings:    SCPlugin-Ifupdown: init!
   | May 10 20:00:48 paranoia nm-system-settings:    SCPlugin-Ifupdown: update_system_hostname
   | May 10 20:00:48 paranoia kernel: [37468.353366] nm-system-setti[20338]: segfault at 0 ip b7ef3c3f sp bfffaa90 error 4 in libdbus-glib-1.so.2.1.0[b7ee8000+1c000]

 * Missing newline at EOF causes the last line to be ignored.

 * If the above segfault is fixed then lines longer than 510 characters have
   their first 510 characters ignored, but the remaining characters are
   interpreted as a seperate line.

In detail:

 77: #define SPACE_OR_TAB(string,ret) {ret = strchr(string,' ');ret=(ret == NULL?strchr(string,'\t'):ret);}

The name suggests this find the first character in string which is either
space or tab.  It actually finds the first space, or failing that the first
tab.  Which is something different when someone seperates the words with tabs
but forgets a space at the end, like this:

  "iface" tab "eth0" tab "inet" tab "dhcp" space newline

See line 113 below.

 78:
 79: void ifparser_init(void)
 80: {
 81:      FILE *inp = fopen(ENI_INTERFACES_FILE, "r");
 82:      int ret = 0;
 83:      char *line;
 84:      char *space;
 85:      char rline[255];
 86:
 87:      if (inp == NULL)
 88:      {
 89:           nm_warning ("Error: Can't open %s\n", ENI_INTERFACES_FILE);
 90:           return;
 91:      }
 92:      first = last = NULL;
 93:      while(1)
 94:      {
 95:           line = space = NULL;
 96:           ret = fscanf(inp,"%255[^\n]\n",rline);

In addition to reading up to 255 characters, fscanf() will append '\0',
requiring rline to be an array of 256 characters.  However, in line 85 rline
has been declared 255 characters long.

Also, that call to fscanf() will never match the last line of a file if the
final '\n' is missing.

 97:           if (ret == EOF)
 98:                break;
 99:           // If the line did not match, skip it
100:           if (ret == 0) {
101:                char *ignored;
102:
103:                ignored = fgets(rline, 255, inp);
104:                continue;
105:           }

Together with the earlier code, this will ignore the first 510 characters of
lines longer than 255 characters.  If a line happens to be longer than 510
characters, the rest will be interpreted as another line.

106:
107:           line = rline;
108:           while(line[0] == ' ')
109:                line++;

The trimming of initial whitespace doesn't work for tabs.

110:           if (line[0]=='#' || line[0]=='\0')
111:                continue;
112:
113:           SPACE_OR_TAB(line,space)

This will _not_ find the first character which is either space or tab (see
line 77 above).

114:                if (space == NULL)
115:                {
116:                     nm_warning ("Error: Can't parse interface line '%s'\n",line);
117:                     continue;
118:                }
119:           space[0] = '\0';
120:
121:           // There are four different stanzas:
122:           // iface, mapping, auto and allow-*. Create a block for each of them.
123:           if (strcmp(line,"iface")==0)
124:           {
125:                char *space2 = strchr(space+1,' ');

This is probably supposed to find the end of a word.  It does not work in the
case where words a seperated by more than one space, because initial spaces
are not skipped.  Tabs are not even considered.

126:                if (space2 == NULL)
127:                {
128:                     nm_warning ("Error: Can't parse iface line '%s'\n",space+1);
129:                     continue;
130:                }
131:                space2[0]='\0';
132:                add_block(line,space+1);
133:
134:                if (space2[1]!='\0')

What is that "if" good for?  This will silently accept something like

  "iface" space "eth0" space newline

and I can't see any other purpose.

135:                {
136:                     space = strchr(space2+1,' ');

See line 125.

137:                     if (space == NULL)
138:                     {
139:                          nm_warning ("Error: Can't parse data '%s'\n",space2+1);
140:                          continue;
141:                     }
142:                     space[0] = '\0';
143:                     add_data(space2+1,space+1);
144:                }
145:           }
146:           else if (strcmp(line,"auto")==0)
147:                add_block(line,space+1);
148:           else if (strcmp(line,"mapping")==0)
149:                add_block(line,space+1);
150:           else if (strncmp(line,"allow-",6)==0)
151:                add_block(line,space+1);
152:           else
153:                add_data(line,space+1);
154:
155:           //printf("line: '%s' ret=%d\n",rline,ret);
156:      }
157:      fclose(inp);
158: }

Thanks,
Jö.

#528095#10
Date:
2009-05-10 20:18:48 UTC
From:
To:
Jö Fahlke wrote:

[..]

Thanks a lot for the detailed bug report. Care to write a patch, now that you
already did the analysis? I'm very busy atm.

Michael

#528095#15
Date:
2009-05-11 07:12:56 UTC
From:
To:
Am Sun, 10. May 2009, 22:18:48 +0200 schrieb Michael Biebl:

I'll have to see whether I find time myself.

Using getline() would make this stuff much easier -- but getline() is a GNU
extension.  So is it permissible to use getline() or would I have to provide a
replacement in case it isn't available?

Thanks,
Jö.

#528095#20
Date:
2009-05-12 08:41:31 UTC
From:
To:
Am Mon, 11. May 2009, 09:12:56 +0200 schrieb Jö Fahlke:

Applied is a first version of a patch to improve ifparser_init().  I didn't
use getline() after all.  It compiles and I did some rudimentary testing.  I'm
still working on an improvement regarding the handling of continued lines.

Comments?

Bye,
Jö.

#528095#25
Date:
2009-05-12 21:37:36 UTC
From:
To:
Am Tue, 12. May 2009, 10:41:31 +0200 schrieb Jö Fahlke:

And here comes the improved version, which I consider final unless someone
cares to comment.

Bye,
Jö.

#528095#32
Date:
2009-05-12 21:59:03 UTC
From:
To:
Jö Fahlke wrote:

Hi Jö,

thanks a lot for the patch.

I'll try to take a look at it on thursday.

Cheers,
Michael

#528095#37
Date:
2010-08-13 07:00:42 UTC
From:
To:
Hi please consider backporting the updated parser for /e/n/i in upstream's git.

Here's the list of fixes the updated parser provides
- it accepts the last line even if it is not terminated by "\n"
- it skips over-long lines, emits a warning and even takes into account
  that over-long lines may be wrapped to next lines
- it un-wraps wrapped lines
- it uses spaces and tabs equivalently to tokenize the input
- it treats "allow-auto" as a synonym to "auto"
- it splits multi-argument "auto"/"allow-*" into multiple
  single-argument stanzas of the same type
- it warns on data stanzas before the first block stanza

Thanks for maintaining network-manager in Debian
Peter

#528095#44
Date:
2013-01-29 14:59:34 UTC
From:
To:
This bug report unfortunately fell completely off my radar.
I apologize for that, especially since you Jö invested time to get a
patch prepared.

Since the patch no longer applies to the current version in wheezy/sid,
would you be willing to update it?

Cheers,
Michael

#528095#49
Date:
2013-01-30 19:33:57 UTC
From:
To:
Am Tue, 29. Jan 2013, 15:59:34 +0100 schrieb Michael Biebl:

Sorry, but sadly I currently don't have the time to look into this.

Regards,
Jö.

#528095#54
Date:
2015-06-15 07:21:49 UTC
From:
To:
Any news on this report? This bug might be responsible for tons of network-manager
related problems.

Regards
Harri