Patchwork [gccgo] Check for overflow when converting from string to int

login
register
mail settings
Submitter Ian Taylor
Date Nov. 17, 2010, 9:56 p.m.
Message ID <mcrr5ejy5qm.fsf@google.com>
Download mbox | patch
Permalink /patch/71624/
State New
Headers show

Comments

Ian Taylor - Nov. 17, 2010, 9:56 p.m.
Joseph Myers pointed out that the Go frontend calls atoi in a few
places, and has a case where it calls strtol without checking for
overflow.  None of these are critical cases.  The calls to atoi are all
when parsing import data which is generated by the compiler.  The strtol
is when parsing a //line comment which sets the line number for debug
info like #line in C.  However, testing for overflow is the right thing
to do anyhow, and this patch implements it.  Committed to gccgo branch.

Ian

Patch

diff -r bd850168de12 go/import.cc
--- a/go/import.cc	Wed Nov 17 11:34:11 2010 -0800
+++ b/go/import.cc	Wed Nov 17 13:52:23 2010 -0800
@@ -311,7 +311,10 @@ 
 
       this->require_c_string("priority ");
       std::string priority_string = this->read_identifier();
-      this->package_->set_priority(atoi(priority_string.c_str()));
+      int prio;
+      if (!this->string_to_int(priority_string, false, &prio))
+	return NULL;
+      this->package_->set_priority(prio);
       this->require_c_string(";\n");
 
       if (stream->match_c_string("import "))
@@ -368,7 +371,9 @@ 
       std::string init_name = this->read_identifier();
       this->require_c_string(" ");
       std::string prio_string = this->read_identifier();
-      int prio = atoi(prio_string.c_str());
+      int prio;
+      if (!this->string_to_int(prio_string, false, &prio))
+	return;
       gogo->add_import_init_fn(package_name, init_name, prio);
     }
   this->require_c_string(";\n");
@@ -480,7 +485,10 @@ 
 	break;
       number += c;
     }
-  int index = atoi(number.c_str());
+
+  int index;
+  if (!this->string_to_int(number, true, &index))
+    return Type::make_error_type();
 
   if (c == '>')
     {
@@ -732,6 +740,24 @@ 
   return ret;
 }
 
+// Turn a string into a integer with appropriate error handling.
+
+bool
+Import::string_to_int(const std::string &s, bool is_neg_ok, int* ret)
+{
+  char* end;
+  long prio = strtol(s.c_str(), &end, 10);
+  if (*end != '\0' || prio > 0x7fffffff || (prio < 0 && !is_neg_ok))
+    {
+      error_at(this->location_, "invalid integer in import data at %d",
+	       this->stream_->pos());
+      this->stream_->set_saw_error();
+      return false;
+    }
+  *ret = prio;
+  return true;
+}
+
 // Class Import::Stream.
 
 Import::Stream::Stream()
diff -r bd850168de12 go/import.h
--- a/go/import.h	Wed Nov 17 11:34:11 2010 -0800
+++ b/go/import.h	Wed Nov 17 13:52:23 2010 -0800
@@ -237,6 +237,10 @@ 
   void
   register_builtin_type(Gogo*, const char* name, Builtin_code);
 
+  // Get an integer from a string.
+  bool
+  string_to_int(const std::string&, bool is_neg_ok, int* ret);
+
   // The general IR.
   Gogo* gogo_;
   // The stream from which to read import data.
diff -r bd850168de12 go/lex.cc
--- a/go/lex.cc	Wed Nov 17 11:34:11 2010 -0800
+++ b/go/lex.cc	Wed Nov 17 13:52:23 2010 -0800
@@ -1599,7 +1599,9 @@ 
 	  if (plend > pcolon + 1
 	      && (plend == pend
 		  || *plend < '0'
-		  || *plend > '9'))
+		  || *plend > '9')
+	      && lineno > 0
+	      && lineno < 0x7fffffff)
 	    {
 	      unsigned int filelen = pcolon - p;
 	      char* file = new char[filelen + 1];