From 69e1295384e7070ef4981745f6aeac6730d1dd3a Mon Sep 17 00:00:00 2001
From: david <david@e0a8ed71-7df4-0310-8962-fdc924857419>
Date: Tue, 9 Nov 2010 19:47:18 +0000
Subject: [PATCH] Change the way ScriptResult::get_id and
 ScriptResult::get_output work to avoid referencing deallocated memory.

The class was defined basically as follows:

class ScriptResult
{
private:
	std::string output;
public:
	std::string get_output() const
	{
		return this->output;
	}
};

The problem was when it was used like this, as in our script output
routines:

const char *s = sr.get_output().c_str();
printf("%s\n", s);

The reason is that the temporary std::string returned by get_output goes
out of scope after the line containing it, which invalidates the memory
pointed to by c_str(). By the time of the printf, s may be pointing to
deallocated memory.

This could have been fixed by returning a const reference that would
remain valid as long as the ScriptResult's output member is valid:

	const std::string& get_output() const
	{
		return this->output;
	}

However I noticed that get_output() was always immediately followed by a
c_str(), so I just had get_output return that instead, which has the
same period of validity.

This problem became visiable when compiling with Visual C++ 2010. The
first four bytes of script output in normal output would be garbage
(probably some kind of free list pointer). It didn't happen in XML
output, because the get_output-returned string happened to remain in
scope during that.
---
 nse_main.cc |  8 ++++----
 nse_main.h  |  4 ++--
 output.cc   | 16 ++++++++--------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/nse_main.cc b/nse_main.cc
index d1e7ac8a2..f41a3a135 100644
--- a/nse_main.cc
+++ b/nse_main.cc
@@ -229,9 +229,9 @@ void ScriptResult::set_output (const char *out)
   output = std::string(out);
 }
 
-std::string ScriptResult::get_output (void) const
+const char *ScriptResult::get_output (void) const
 {
-  return output;
+  return output.c_str();
 }
 
 void ScriptResult::set_id (const char *ident)
@@ -239,9 +239,9 @@ void ScriptResult::set_id (const char *ident)
   id = std::string(ident);
 }
 
-std::string ScriptResult::get_id (void) const
+const char *ScriptResult::get_id (void) const
 {
-  return id;
+  return id.c_str();
 }
 
 ScriptResults *get_script_scan_results_obj (void)
diff --git a/nse_main.h b/nse_main.h
index 95bb810e3..3a1455d3e 100644
--- a/nse_main.h
+++ b/nse_main.h
@@ -23,9 +23,9 @@ class ScriptResult
     std::string id;
   public:
     void set_output (const char *);
-    std::string get_output (void) const;
+    const char *get_output (void) const;
     void set_id (const char *);
-    std::string get_id (void) const;
+    const char *get_id (void) const;
 };
 
 typedef std::list<ScriptResult> ScriptResults;
diff --git a/output.cc b/output.cc
index 22ca8b46d..c46e44fc7 100644
--- a/output.cc
+++ b/output.cc
@@ -425,7 +425,7 @@ static char *formatScriptOutput(ScriptResult sr) {
   std::string result;
   unsigned int i;
 
-  c_output = sr.get_output().c_str();
+  c_output = sr.get_output();
   p = c_output;
 
   while (*p != '\0') {
@@ -447,7 +447,7 @@ static char *formatScriptOutput(ScriptResult sr) {
     else
       result += "|_";
     if (i == 0)
-      result += sr.get_id() + ": ";
+      result += std::string(sr.get_id()) + ": ";
     result += lines[i];
     if (i < lines.size() - 1)
       result += "\n";
@@ -787,8 +787,8 @@ void printportoutput(Target *currenths, PortList *plist) {
           for (ssr_iter = current->scriptResults.begin();
                ssr_iter != current->scriptResults.end(); ssr_iter++) {
             xml_open_start_tag("script");
-            xml_attribute("id", "%s", ssr_iter->get_id().c_str());
-            xml_attribute("output", "%s", ssr_iter->get_output().c_str());
+            xml_attribute("id", "%s", ssr_iter->get_id());
+            xml_attribute("output", "%s", ssr_iter->get_output());
             xml_close_empty_tag();
 
             char *script_output = formatScriptOutput((*ssr_iter));
@@ -1943,8 +1943,8 @@ void printscriptresults(ScriptResults *scriptResults, stype scantype) {
          iter != scriptResults->end();
          iter++) {
       xml_open_start_tag("script");
-      xml_attribute("id", "%s", iter->get_id().c_str());
-      xml_attribute("output", "%s", iter->get_output().c_str());
+      xml_attribute("id", "%s", iter->get_id());
+      xml_attribute("output", "%s", iter->get_output());
       xml_close_empty_tag();
       script_output = formatScriptOutput((*iter));
       log_write(LOG_PLAIN, "%s\n", script_output);
@@ -1965,8 +1965,8 @@ void printhostscriptresults(Target *currenths) {
          iter != currenths->scriptResults.end();
          iter++) {
       xml_open_start_tag("script");
-      xml_attribute("id", "%s", iter->get_id().c_str());
-      xml_attribute("output", "%s", iter->get_output().c_str());
+      xml_attribute("id", "%s", iter->get_id());
+      xml_attribute("output", "%s", iter->get_output());
       xml_close_empty_tag();
       script_output = formatScriptOutput((*iter));
       log_write(LOG_PLAIN, "%s\n", script_output);