[issue25723] ConfigParser should never write broken configurations

SpaceOne added the comment: Of course both of you have reasonable arguments. For compatibility with overridden SECTRE attributes it should not raise ValueError for characters like [ and ]. (too bad that SECTRE is a public attribute otherwise it could also be used to validate the name (SECTRE.m

[issue25723] ConfigParser should never write broken configurations

R. David Murray added the comment: I view this as similar to the corresponding issue with email headers, where we fixed a similar security issue. The special danger of \n is that it allows you to create a *new* header, or in this case section, with an arbitrary value, possibly overriding an e

[issue25723] ConfigParser should never write broken configurations

Terry J. Reedy added the comment: We all know that blindly inserting external data into a database can be a bad idea. But raising ValueError if the data contains \n barely scratches the surface of a real defense. The external data should be checked before passing it to .add_section or as par

[issue25723] ConfigParser should never write broken configurations

R. David Murray added the comment: The issue would be if the section name came from user input. Then an attacker could craft a section name that would result in inserting arbitrary names and values into the config file. -- nosy: +r.david.murray ___

[issue25723] ConfigParser should never write broken configurations

Terry J. Reedy added the comment: Can you explain how passing \n createda vulnerability (to who, doing what) that flagging \n would prevent? Your opening example (nicely presented, by the way) shows that passing \n allows one to do with one call what would otherwise take (in the case of the e

[issue25723] ConfigParser should never write broken configurations

SpaceOne added the comment: Isn't is an actual problem in the field? We had a vulnerability in our code due to this as we only sanitized the config values and didn't recognized that add_section() does no validation of input. -- ___ Python tracker <

[issue25723] ConfigParser should never write broken configurations

Terry J. Reedy added the comment: Newline (\n) and possibly \x00, if it necessarily causes an actual problem, are the only characters that we might reject by default. Rejecting anything else is unwarrented editorializing about what people 'should' use. As you said, people who want something

[issue25723] ConfigParser should never write broken configurations

SpaceOne added the comment: I added also a rejection of '\r' and '\x00': https://github.com/spaceone/cpython/commit/41d6e278e4ffa95577d843e0d50d4c43b5ac46ef It's only my opinion but I would prefer to reject all of these non printable characters: '\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c

[issue25723] ConfigParser should never write broken configurations

STINNER Victor added the comment: Terry: "Since anything else without ']' is valid (...)" A Python script can be used to generate a configuration read by another application. This application can more more strict on the configuration format than Python, so I would prefer to deny '\n', '[' and

[issue25723] ConfigParser should never write broken configurations

Terry J. Reedy added the comment: This strikes me as an overt bug. .add_section should add one new empty section, not a section with an item and a second section. Since a section name cannot contain \n, I would agree that passing a string containing \n should raise ValueError("Section names

[issue25723] ConfigParser should never write broken configurations

SpaceOne added the comment: I would have expected something like ValueError('A section must not contain any of ...') or at least that the characters are simply stripped. -- ___ Python tracker _

[issue25723] ConfigParser should never write broken configurations

New submission from SpaceOne: >>> from configparser import ConfigParser >>> from io import StringIO >>> from configparser import ConfigParser >>> c = ConfigParser() >>> c.add_section('foo]\nbar=baz\n[bar') >>> fd = StringIO() >>> c.write(fd) >>> print(fd.getvalue()) [foo] bar=baz [bar] User inpu

[issue25723] ConfigParser should never write broken configurations

Changes by R. David Murray : -- nosy: +lukasz.langa versions: -Python 3.2, Python 3.3, Python 3.4 ___ Python tracker ___ ___ Python-b