René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Unhandled IndexError when calling .read() on a malformed config file","articleBody":"Hi, I noticed the fuzzing tests that OSS-Fuzz runs on this project are broken and while I was working on fixing them I believe I came across a minor bug:\r\n\r\n## The Bug\r\n\r\nAn `Uncaught Python exception:` `IndexError: string index out of range` can be triggered if when trying to call `.read()` on `GitConfigParser` if it was initialized with a malformed config file.\r\n\r\n### Current Behavior\r\n\r\nIt's easiest to demonstrate, so please consider this example:\r\n\r\n```python\r\nimport io\r\nfrom git.config import GitConfigParser\r\n\r\n\r\ndef reproduce_issue():\r\n\r\n    malformed_config_content_bytestring = b'[-]\\nk:\"v\\n\"'\r\n\r\n    problematic_config_file = io.BytesIO(malformed_config_content_bytestring)\r\n\r\n    # problematic_config_file looks now like this:\r\n    \"\"\"\r\n    [-]\r\n    k:\"v\r\n    \"\r\n    \"\"\"\r\n\r\n    # We have to name the file otherwise we'll trigger\r\n    # `AttributeError: '_io.BytesIO' object has no attribute 'name'` here:\r\n    # https://github.com/gitpython-developers/GitPython/blob/c7675d2cedcd737f20359a4a786e213510452413/git/config.py#L623\r\n    problematic_config_file.name = \"fuzzedconfig.config\"\r\n\r\n    # This is fine\r\n    git_config = GitConfigParser(problematic_config_file)\r\n    \r\n    # The next line raised an unhandled `IndexError: string index out of range`\r\n    git_config.read()\r\n\r\n\r\nif __name__ == \"__main__\":\r\n    reproduce_issue()\r\n\r\n```\r\n\r\nAssuming that code is in `/path/to/example/config_indexerror_reproduction.py` then\r\n```sh\r\npython config_indexerror_reproduction.py\r\n```\r\nproduces something akin to:\r\n\r\n```sh\r\nTraceback (most recent call last):\r\n  File \"/path/to/example/config_indexerror_reproduction.py\", line 30, in \u003cmodule\u003e\r\n    reproduce_issue()\r\n  File \"/path/to/example/config_indexerror_reproduction.py\", line 26, in reproduce_issue\r\n    git_config.read()\r\n  File \"/path/to/example/.venv/lib/python3.12/site-packages/git/config.py\", line 607, in read\r\n    self._read(file_path, file_path.name)\r\n  File \"/path/to/example/.venv/lib/python3.12/site-packages/git/config.py\", line 514, in _read\r\n    cursect.setlast(optname, optval + string_decode(line))\r\n                                      ^^^^^^^^^^^^^^^^^^^\r\n  File \"/path/to/example/.venv/lib/python3.12/site-packages/git/config.py\", line 441, in string_decode\r\n    if v[-1] == \"\\\\\":\r\n       ~^^^^\r\nIndexError: string index out of range\r\n```\r\n\r\n\u003cdetails\u003e\u003csummary\u003eMy Reproduction Environment Details\u003c/summary\u003e\r\n\r\nThe reproduction code above was tested on:\r\n\r\n```sh\r\nPython Version: 3.12.1 (main, Feb  5 2024, 16:23:00) [Clang 15.0.0 (clang-1500.1.0.2.5)]\r\nOS Information: macOS-14.4.1-x86_64-i386-64bit\r\nInstalled Packages:\r\nPackage   Version\r\n--------- -------\r\ngitdb     4.0.11\r\nGitPython 3.1.42\r\npip       24.0\r\nsmmap     5.0.1\r\n```\r\n\r\nAnd the fuzzer environment was:\r\n```\r\nPython Version: 3.8.3 (default, Mar 17 2024, 03:21:27)\r\n[Clang 15.0.0 (https://github.com/llvm/llvm-project.git bf7f8d6fa6f460bf0a16ffe\r\nOS Information: Linux-6.6.16-linuxkit-x86_64-with-glibc2.2.5\r\nInstalled Packages:\r\nPackage                   Version\r\n------------------------- -------\r\naltgraph                  0.17.4\r\natheris                   2.3.0\r\ncoverage                  6.3.2\r\nimportlib_metadata        7.0.2\r\ngitdb                     4.0.11\r\nGitPython                 3.1.42\r\npip                       24.0\r\nsmmap                     5.0.1\r\npackaging                 24.0\r\npyinstaller               5.0.1\r\nsetuptools                41.0.1\r\nsix                       1.15.0\r\nzipp                      3.18.1\r\n```\r\n\r\n\u003c/details\u003e \r\n\r\nSo, if I'm reading the source correct, it seems like the combination of some header section (`[-]` above) followed by a key/value assignment that has a value consisting of a double quoted string with a new line inside it confuses the check here which strips the `\"` on the new line:\r\nhttps://github.com/gitpython-developers/GitPython/blob/c7675d2cedcd737f20359a4a786e213510452413/git/config.py#L521-L528\r\n\r\nand passes an empty string to `string_decode` which isn't expecting that when it indexes into it's arg:\r\nhttps://github.com/gitpython-developers/GitPython/blob/c7675d2cedcd737f20359a4a786e213510452413/git/config.py#L454-L455\r\n\r\n### Expected Behavior\r\n\r\nI'd expect an explicitly raised `ParsingError` similar to how it's handled a little further up:\r\nhttps://github.com/gitpython-developers/GitPython/blob/c7675d2cedcd737f20359a4a786e213510452413/git/config.py#L514-L518\r\n\r\n","author":{"url":"https://github.com/DaveLak","@type":"Person","name":"DaveLak"},"datePublished":"2024-03-30T19:37:43.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":6},"url":"https://github.com/1887/GitPython/issues/1887"}