git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH maint 0/3] do not write files outside of work-dir

From
TTait <git.git@t41t.com>
Date
Jun 1, 2011, 04:14 UTC
Message-ID
<20110601041439.GH29958@ece.pdx.edu>
In-Reply-To
<1306512040-1468-1-git-send-email-kusmabite@gmail.com>
> Theo Niessink has uncovered a serious sercurity issue in Git for Windows,
> where cloning an evil repository can arbitrarily overwrite files outside
> the repository...

Filenames starting with C: are not necessarily absolute. Consider "c:foo.txt" where c: is the current directory on drive C, or "c:stream1" where c is a single-letter filename in the current directory with an alternate data stream such as would be shown by dir /r. The has_dos_drive_prefix check is overly broad. Maybe this is intentional and just needs to be documented. Absolute paths like \\localhost\C$\file.txt and \\?\C:\file.txt do seem to be caught, because they start with '\'.

Microsoft says[1] a path is relative unless:
  - it begins with "\\"
  - it begins with a disk designator followed by a directory separator
  - it begins with a single "\"
On that basis, has_dos_drive_prefix(path) should be:
  isalpha(*(path)) && (path)[1] == ':' && is_dir_sep((path)[2])

However, there are also paths within the NT namespace (as opposed to the Win32 namespace, [1] again) that might be considered absolute, or at least to which git should not try to write. Examples would be PRN, CONOUT$, AUX, etc. These will not be caught by the current form of has_dos_drive_prefix, if that is even the right place to catch them. I think the QueryDosDevice function (given the part of the path up to the first directory separator, if one is present [2]) would detect them, and logical drive mappings as well. However, QueryDosDevice seems to also include many things that are not worthy of concern, like (on my computer) "DISPLAY5". Does anyone know the correct approach here?

I gather that other programs can create names like these (with DefineDosDevice), so a hard-coded exception list from [1] (that being: CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7, COM8, COM9, LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, and LPT9) might not be adequate?

[1] http://msdn.microsoft.com/en-us/library/aa365247(v=vs.85).aspx [2] http://msdn.microsoft.com/en-us/library/aa365461(v=vs.85).aspx

Previous: Junio C HamanoNext: Johannes Sixt
Message 19 of 20 in “do not write files outside of work-dir”
  1. 0/3 do not write files outside of work-dirErik Faye-Lund, May 27, 2011
  2. 1/3 A Windows path starting with a backslash is absoluteErik Faye-Lund, May 27, 2011
  3. 2/3 real_path: do not assume '/' is the path seperatorErik Faye-Lund, May 27, 2011
  4. 3/3 verify_path: consider dos drive prefixErik Faye-Lund, May 27, 2011
  5. Johannes SixtMay 27, 2011
  6. Erik Faye-LundMay 30, 2011
  7. Theo NiessinkMay 30, 2011
  8. Erik Faye-LundMay 30, 2011
  9. Junio C HamanoJun 7, 2011
  10. Erik Faye-LundJun 7, 2011
  11. Erik Faye-LundJun 7, 2011
  12. Junio C HamanoJun 7, 2011
  13. Erik Faye-LundJun 7, 2011
  14. Theo NiessinkJun 7, 2011
  15. Johannes SixtMay 30, 2011
  16. Junio C HamanoMay 27, 2011
  17. Johannes SchindelinMay 27, 2011
  18. Junio C HamanoMay 27, 2011
  19. TaitJun 1, 2011
  20. Johannes SixtJun 1, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.