5 ms·

file is indeed a pretty simple program. Why allow it to do more than it needs to?

by DasIch 12y ago

raldi 12y ago
http://en.wikipedia.org/wiki/Small_matter_of_programming http://en.wikipedia.org/wiki/Small_matter_of_programming
- Dylan16807 12y ago
  I think you meant to link to http://en.wikipedia.org/wiki/Software_bloat http://en.wikipedia.org/wiki/Software_bloat All file needs to do is scan for some magic bytestrings, and optionally print the numbers at a handful of offsets. It currently does much more than that, which is why it's insecure and hard to fix.
  - raldi 12y ago
    You say "needs" as if it's a matter of fact, but what a program needs to do is a very subjective matter. If you feel the command does more than it needs to, could you call out a few examples of bloated features that you would cut?
    - Dylan16807 12y ago
      Okay, I appear to have misremembered a problem in "strings" as being in "file", where it went overboard in parsing and introduced vulnerabilities. But I haven't seen anything to disagree with file being similarly problematic. A quote like To sum up: If somebody uses 'file' in an unconstrained OS environment on untrusted inputs, and he gets pwnd in the result, then it's not a security problem, it's an incompetence problem - and IMO it should be discussed elsewhere. does not suggest that the program is very well designed. Scanning for byte strings with no possibility of security flaw is a solved problem.
- IvyMike 12y ago
  I think DasIch is saying "why allow file(1) to open sockets, write to arbitrary files, and run external programs"? Given the correct input, at least a month ago, it could do all of those things. (I am not sure that attempting to enforce this within the file(1) binary is optimal... after all, even though the attack surface is much reduced, file(1) could still have a bug somewhere prior to the sandboxing. If you could do a "chpriv -write_to_disk -socket -run_external_program /bin/file" that the OS would enforce, that would be cool. Someone should create that.)
  - raldi 12y ago
    > why allow file() to open sockets If by "open sockets" you mean open existing sockets in read-only mode, it's so that it can identify them as sockets. If by "open sockets" you mean create new sockets, I don't think it does do that: https://github.com/threatstack/libmagic/search?utf8=%E2%9C%93&q=socket https://github.com/threatstack/libmagic/search?utf8=%E2%9C%9... > write to arbitrary files It appears it only does this if running on OS/2 and investigating what's inside a compressed file. Under these conditions, a temporary file is necessary for platform-specific reasons: https://github.com/threatstack/libmagic/blob/3dea7072b8d7e927e6f9b1ef381a70271376f322/src/file.h#L466 https://github.com/threatstack/libmagic/blob/3dea7072b8d7e92... https://github.com/threatstack/libmagic/blob/3dea7072b8d7e927e6f9b1ef381a70271376f322/src/apptype.c#L68 https://github.com/threatstack/libmagic/blob/3dea7072b8d7e92... It also writes to a non-arbitrary mmapped file (the magic database), because that's how such databases work; you query them by writing to them in a particular way: https://github.com/threatstack/libmagic/blob/3dea7072b8d7e927e6f9b1ef381a70271376f322/src/apprentice.c#L2782 https://github.com/threatstack/libmagic/blob/3dea7072b8d7e92... > run external programs I can't find any examples where it does that. Do you know of any?
    - IvyMike 12y ago
      "It may be possible for such an attacker to execute arbitrary code with the privileges of the user running file(1)." This is what I am saying...given the right input, file(1) could do anything and everything. Yes, it's only due to a bug in file(1), but still that's kind of ridiculous. We have all sorts of things in place to protect against other bugs (for example, segmentation faults), and there's 27 years of evidence that we need some more help.
DominikD 12y ago
It's a configurable parser. Parsers tend to be the hardest thing to get right, hence bugs detected by AFL in FreeBSD's file, in SQL parser in SQLite, etc. A lot of the vulnerabilities in apps dealing with image files come down to parser being buggy. It seems simple until you actually try and implement it.
- alnsn 12y ago
  It depends on how much effort you put in making sure your parser is robust. I ran AFL tests for several days trying to find bugs in Lua parser but AFL kept discovering a way to load a binary chunk. After that, it didn't take to crash on a malformed binary chunk. Priv separation is a good idea even if you trust your parser.
  - DominikD 12y ago
    This is true for any piece of code but you wouldn't say that it about, say, httpd (oh, priv separation isn't needed if you put enough effort in making sure your protocol implementation is robust). Plus: file (well, magic) is a configurable parser.
bandrami 12y ago
http://en.wikipedia.org/wiki/Halting_problem http://en.wikipedia.org/wiki/Halting_problem Welcome to the joys of parsing.

file is indeed a pretty simple program. Why allow it to do more than it needs to?

by DasIch 12y ago

file is indeed a pretty simple program. Why allow it to do more than it needs to?

raldi 12y ago

http://en.wikipedia.org/wiki/Small_matter_of_programming http://en.wikipedia.org/wiki/Small_matter_of_programming

Dylan16807 12y ago
I think you meant to link to http://en.wikipedia.org/wiki/Software_bloat http://en.wikipedia.org/wiki/Software_bloat All file needs to do is scan for some magic bytestrings, and optionally print the numbers at a handful of offsets. It currently does much more than that, which is why it's insecure and hard to fix.
- raldi 12y ago
  You say "needs" as if it's a matter of fact, but what a program needs to do is a very subjective matter. If you feel the command does more than it needs to, could you call out a few examples of bloated features that you would cut?
  - Dylan16807 12y ago
    Okay, I appear to have misremembered a problem in "strings" as being in "file", where it went overboard in parsing and introduced vulnerabilities. But I haven't seen anything to disagree with file being similarly problematic. A quote like To sum up: If somebody uses 'file' in an unconstrained OS environment on untrusted inputs, and he gets pwnd in the result, then it's not a security problem, it's an incompetence problem - and IMO it should be discussed elsewhere. does not suggest that the program is very well designed. Scanning for byte strings with no possibility of security flaw is a solved problem.
IvyMike 12y ago
I think DasIch is saying "why allow file(1) to open sockets, write to arbitrary files, and run external programs"? Given the correct input, at least a month ago, it could do all of those things. (I am not sure that attempting to enforce this within the file(1) binary is optimal... after all, even though the attack surface is much reduced, file(1) could still have a bug somewhere prior to the sandboxing. If you could do a "chpriv -write_to_disk -socket -run_external_program /bin/file" that the OS would enforce, that would be cool. Someone should create that.)
- raldi 12y ago
  > why allow file() to open sockets If by "open sockets" you mean open existing sockets in read-only mode, it's so that it can identify them as sockets. If by "open sockets" you mean create new sockets, I don't think it does do that: https://github.com/threatstack/libmagic/search?utf8=%E2%9C%93&q=socket https://github.com/threatstack/libmagic/search?utf8=%E2%9C%9... > write to arbitrary files It appears it only does this if running on OS/2 and investigating what's inside a compressed file. Under these conditions, a temporary file is necessary for platform-specific reasons: https://github.com/threatstack/libmagic/blob/3dea7072b8d7e927e6f9b1ef381a70271376f322/src/file.h#L466 https://github.com/threatstack/libmagic/blob/3dea7072b8d7e92... https://github.com/threatstack/libmagic/blob/3dea7072b8d7e927e6f9b1ef381a70271376f322/src/apptype.c#L68 https://github.com/threatstack/libmagic/blob/3dea7072b8d7e92... It also writes to a non-arbitrary mmapped file (the magic database), because that's how such databases work; you query them by writing to them in a particular way: https://github.com/threatstack/libmagic/blob/3dea7072b8d7e927e6f9b1ef381a70271376f322/src/apprentice.c#L2782 https://github.com/threatstack/libmagic/blob/3dea7072b8d7e92... > run external programs I can't find any examples where it does that. Do you know of any?
  - IvyMike 12y ago
    "It may be possible for such an attacker to execute arbitrary code with the privileges of the user running file(1)." This is what I am saying...given the right input, file(1) could do anything and everything. Yes, it's only due to a bug in file(1), but still that's kind of ridiculous. We have all sorts of things in place to protect against other bugs (for example, segmentation faults), and there's 27 years of evidence that we need some more help.

DominikD 12y ago

It's a configurable parser. Parsers tend to be the hardest thing to get right, hence bugs detected by AFL in FreeBSD's file, in SQL parser in SQLite, etc. A lot of the vulnerabilities in apps dealing with image files come down to parser being buggy. It seems simple until you actually try and implement it.

alnsn 12y ago
It depends on how much effort you put in making sure your parser is robust. I ran AFL tests for several days trying to find bugs in Lua parser but AFL kept discovering a way to load a binary chunk. After that, it didn't take to crash on a malformed binary chunk. Priv separation is a good idea even if you trust your parser.
- DominikD 12y ago
  This is true for any piece of code but you wouldn't say that it about, say, httpd (oh, priv separation isn't needed if you put enough effort in making sure your protocol implementation is robust). Plus: file (well, magic) is a configurable parser.

bandrami 12y ago

http://en.wikipedia.org/wiki/Halting_problem http://en.wikipedia.org/wiki/Halting_problem Welcome to the joys of parsing.