execscan: detect arbitrary file open #8009

catenacyber · 2022-07-14T13:39:29Z

oliverchang · 2022-07-15T04:24:44Z

infra/experimental/sanitizers/ExecSan/execSan.cpp

@@ -66,6 +66,10 @@ const std::string kTripWire = "/tmp/tripwire";
 const std::string kInjectionError = "Shell injection";
 // Shell corruption bug detected based on syntax error.
 const std::string kCorruptionError = "Shell corruption";
+// The magic string that we'll use to detect arbitrary file open
+const std::string kFzAbsoluteDirectory = "/fz/";


Nice! I wonder if we could gneralize this though to be able to catch these without having to rely on a fixed string.

Maybe we could report a bug on any access on a root /dir that doesn't exist?

CC @alan32liu @jonathanmetzman

Yep, it sounds like a good start for a sanitizer!
One similar idea come across Jonathan and I while we were brainstorming yesterday was to secretly create some files in common dirs (etc/) without telling the program under test and check if any of them are opened.
We could also use dictionaries if we want to check a particular file.

How to avoid false positive remains an open questions.

Maybe we could report a bug on any access on a root /dir that doesn't exist?

Just pushed that @oliverchang

secretly create some files in common dirs (etc/) without telling the program under test and check if any of them are opened.

Like if some program lists all the files in a directory and opens them ?
Not sure about the attack scenario behind this...

How to avoid false positive remains an open questions

I think this is a per fuzz-target config option
My yaml fuzz target will try to open arbitrary files with some include command and it is ok
But I do not think my png or pcap fuzz target should do that...

Maybe we could report a bug on any access on a root /dir that doesn't exist?

Has anyone tested to see if this isn't normal? If / is in a program's PATH (not super common, but a reasonable example IMO), calling Popen will always look for files that don't exist in / to try to execute them.

I don't think there should be a trailing slash.

Has anyone tested to see if this isn't normal? If / is in a program's PATH (not super common, but a reasonable example IMO), calling Popen will always look for files that don't exist in / to try to execute them.

It seems pretty rare that '/' would be in a program's PATH, and even if it is, it'd probably go through stat first before we call open (if at all -- we'll probably just call execve on it instead). We could also check that it's a dir, and not any random file.

Either way, we can experiment and see what results we get back. I expect projects will need some config/opt-out mechanism for the things we add (even for the command injection one).

jonathanmetzman · 2022-07-18T20:12:17Z

infra/experimental/sanitizers/ExecSan/execSan.cpp

@@ -66,6 +66,10 @@ const std::string kTripWire = "/tmp/tripwire";
 const std::string kInjectionError = "Shell injection";
 // Shell corruption bug detected based on syntax error.
 const std::string kCorruptionError = "Shell corruption";
+// The magic string that we'll use to detect arbitrary file open
+const std::string kFzAbsoluteDirectory = "/fz/";


Maybe we could report a bug on any access on a root /dir that doesn't exist?

Has anyone tested to see if this isn't normal? If / is in a program's PATH (not super common, but a reasonable example IMO), calling Popen will always look for files that don't exist in / to try to execute them.

jonathanmetzman · 2022-07-18T20:12:51Z

infra/experimental/sanitizers/ExecSan/execSan.cpp

+const std::string kFzAbsoluteDirectory = "/fz/";
+// Arbitrary file open in /fz/
+const std::string kArbitraryFileOpenError = "Arbitrary file open";
+// Assuming we will a shorter top dir.


This comment isn't complete. I'm not sure what it is trying to say.

improved it indeed.

jonathanmetzman · 2022-07-18T20:13:08Z

infra/experimental/sanitizers/ExecSan/execSan.cpp

@@ -66,6 +66,10 @@ const std::string kTripWire = "/tmp/tripwire";
 const std::string kInjectionError = "Shell injection";
 // Shell corruption bug detected based on syntax error.
 const std::string kCorruptionError = "Shell corruption";
+// The magic string that we'll use to detect arbitrary file open
+const std::string kFzAbsoluteDirectory = "/fz/";


I don't think there should be a trailing slash.

jonathanmetzman · 2022-07-18T20:13:49Z

infra/experimental/sanitizers/ExecSan/execSan.cpp

+    if (found != std::string::npos) {
+      std::string path_absolute_topdir = path.substr(0, found);
+      struct stat dirstat;
+      if (stat (path_absolute_topdir.c_str(), &dirstat) != 0) {


delete the space after stat

jonathanmetzman · 2022-07-18T20:14:14Z

infra/experimental/sanitizers/ExecSan/target.cpp

 #include <string>
 #include <iostream>

 extern "C" int LLVMFuzzerTestOneInput(char* data, size_t size) {
  std::string str(data, size);
  std::cout << "INPUT" << str << std::endl;
  system(str.c_str());
+  FILE *fp = fopen(str.c_str(), "r");


This should probably be in a seperate target?

+1 let's create a new one rather than re-use this one.

oliverchang

@catenacyber mind addressing the comments?

I've created google/clusterfuzz#2739 so we should be able to experiment with crazier checks in a way that doesn't bother oss-fuzz project maintainers.

catenacyber · 2022-07-22T06:01:52Z

I will look into these next week

catenacyber · 2022-07-25T09:32:38Z

@oliverchang I just pushed an update

oliverchang · 2022-08-02T07:10:28Z

infra/experimental/sanitizers/ExecSan/execSan.cpp

+    report_bug(kArbitraryFileOpenError);
+  }
+  if (path[0] == '/' && path.length() > 1) {
+    size_t found = path.find('/', 1);


nit: s/found/root_dir_end/

oliverchang · 2022-08-02T07:12:59Z

infra/experimental/sanitizers/ExecSan/execSan.cpp

+  }
+  if (path[0] == '/' && path.length() > 1) {
+    size_t found = path.find('/', 1);
+    if (found != std::string::npos) {


if found == std::string::npos, can we just take the entire string?

i.e. we should be able to detect both /blahdgfdf and /blahdgfdf/. Handling the former makes it slightly easier to discover.

catenacyber · 2022-08-02T08:26:59Z

Ci failure seems to be a network failure

oliverchang · 2022-08-03T03:10:15Z

Thanks @catenacyber. I've opened google/clusterfuzz#2750 to track the required ClusterFuzz-side work to get this working.

* execscan: detect arbitrary file open * Checks for unknown top dir * move the file open test to its own fuzz target * Fixups from PR review

execscan: detect arbitrary file open

cd7caf4

oliverchang reviewed Jul 15, 2022

View reviewed changes

Checks for unknown top dir

cb1075c

jonathanmetzman reviewed Jul 18, 2022

View reviewed changes

oliverchang reviewed Jul 22, 2022

View reviewed changes

move the file open test to its own fuzz target

525829a

oliverchang requested review from jonathanmetzman, DonggeLiu and oliverchang August 2, 2022 06:58

oliverchang requested changes Aug 2, 2022

View reviewed changes

Fixups from PR review

61307c5

oliverchang approved these changes Aug 3, 2022

View reviewed changes

oliverchang merged commit 2964ab1 into google:master Aug 3, 2022

oliverchang mentioned this pull request Aug 3, 2022

Add support for parsing arbitrary file open google/clusterfuzz#2750

Closed

DonggeLiu mentioned this pull request Aug 11, 2022

Support parsing arbitrary file open google/clusterfuzz#2759

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

execscan: detect arbitrary file open #8009

execscan: detect arbitrary file open #8009

catenacyber commented Jul 14, 2022

oliverchang Jul 15, 2022

DonggeLiu Jul 15, 2022

catenacyber Jul 15, 2022

jonathanmetzman Jul 18, 2022

jonathanmetzman Jul 18, 2022

oliverchang Jul 19, 2022

jonathanmetzman Jul 18, 2022

jonathanmetzman Jul 18, 2022

catenacyber Jul 25, 2022

jonathanmetzman Jul 18, 2022

jonathanmetzman Jul 18, 2022

catenacyber Jul 25, 2022

jonathanmetzman Jul 18, 2022

oliverchang Jul 19, 2022

oliverchang left a comment

catenacyber commented Jul 22, 2022

catenacyber commented Jul 25, 2022

oliverchang Aug 2, 2022

catenacyber Aug 2, 2022

oliverchang Aug 2, 2022

catenacyber Aug 2, 2022

catenacyber commented Aug 2, 2022

oliverchang commented Aug 3, 2022

execscan: detect arbitrary file open #8009

execscan: detect arbitrary file open #8009

Conversation

catenacyber commented Jul 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliverchang left a comment

Choose a reason for hiding this comment

catenacyber commented Jul 22, 2022

catenacyber commented Jul 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catenacyber commented Aug 2, 2022

oliverchang commented Aug 3, 2022