From patchwork Wed Nov 17 21:56:01 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 71624 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2D179B7161 for ; Thu, 18 Nov 2010 08:56:18 +1100 (EST) Received: (qmail 2604 invoked by alias); 17 Nov 2010 21:56:16 -0000 Received: (qmail 2591 invoked by uid 22791); 17 Nov 2010 21:56:15 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, TW_CC, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 17 Nov 2010 21:56:09 +0000 Received: from hpaq5.eem.corp.google.com (hpaq5.eem.corp.google.com [172.25.149.5]) by smtp-out.google.com with ESMTP id oAHLu7Wq025829 for ; Wed, 17 Nov 2010 13:56:07 -0800 Received: from gwb17 (gwb17.prod.google.com [10.200.2.17]) by hpaq5.eem.corp.google.com with ESMTP id oAHLtTHJ006534 for ; Wed, 17 Nov 2010 13:56:05 -0800 Received: by gwb17 with SMTP id 17so1530498gwb.30 for ; Wed, 17 Nov 2010 13:56:05 -0800 (PST) Received: by 10.90.228.10 with SMTP id a10mr12282311agh.176.1290030965394; Wed, 17 Nov 2010 13:56:05 -0800 (PST) Received: from coign.google.com (dhcp-172-22-124-218.mtv.corp.google.com [172.22.124.218]) by mx.google.com with ESMTPS id e47sm1879242yhc.4.2010.11.17.13.56.03 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 17 Nov 2010 13:56:04 -0800 (PST) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com Subject: [gccgo] Check for overflow when converting from string to int Date: Wed, 17 Nov 2010 13:56:01 -0800 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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];