Check the return value of base::StringToInt() in

net/proxy/proxy_config_service_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/proxy/proxy_config_service_linux.h"
 
 #include <errno.h>
 #if defined(USE_GCONF)
 #include <gconf/gconf-client.h>
 #endif  // defined(USE_GCONF)
@@ skipping 838 matching lines @@
       VLOG(1) << "Found gnome-network-properties. Will fall back to gconf.";
       return false;
     }
   }
 
   VLOG(1) << "All gsettings tests OK. Will get proxy config from gsettings.";
   return true;
 }
 #endif  // defined(USE_GIO)
 
+// Converts |value| from a decimal string to an int. If there was a failure
+// parsing, returns |default_value|.
+int StringToIntOrDefault(base::StringPiece value, int default_value) {
+  int result;
+  if (base::StringToInt(value, &result))
+    return result;
+  return default_value;
+}
+
 // This is the KDE version that reads kioslaverc and simulates gconf.
 // Doing this allows the main Delegate code, as well as the unit tests
 // for it, to stay the same - and the settings map fairly well besides.
 class SettingGetterImplKDE : public ProxyConfigServiceLinux::SettingGetter {
  public:
   explicit SettingGetterImplKDE(base::Environment* env_var_getter)
       : inotify_fd_(-1),
         notify_delegate_(nullptr),
         debounce_timer_(new base::OneShotTimer()),
         indirect_manual_(false),
@@ skipping 212 matching lines @@
         tokens.push_back(token);
     }
     strings_table_[key] = tokens;
   }
 
   void AddKDESetting(const std::string& key, const std::string& value) {
     if (key == "ProxyType") {
       const char* mode = "none";
       indirect_manual_ = false;
       auto_no_pac_ = false;
-      int int_value;
-      base::StringToInt(value, &int_value);
+      int int_value = StringToIntOrDefault(value, 0);
       switch (int_value) {
-        case 0:  // No proxy, or maybe kioslaverc syntax error.
-          break;
         case 1:  // Manual configuration.
           mode = "manual";
           break;
         case 2:  // PAC URL.
           mode = "auto";
           break;
         case 3:  // WPAD.
           mode = "auto";
           auto_no_pac_ = true;
           break;
         case 4:  // Indirect manual via environment variables.
           mode = "manual";
           indirect_manual_ = true;
           break;
+        default:  // No proxy, or maybe kioslaverc syntax error.

[eroman, 2017/06/29 21:11:18]

(Previously didn't account for negative numbers)

+          break;
       }
       string_table_[PROXY_MODE] = mode;
     } else if (key == "Proxy Config Script") {
       string_table_[PROXY_AUTOCONF_URL] = value;
     } else if (key == "httpProxy") {
       AddProxy(PROXY_HTTP_HOST, value);
     } else if (key == "httpsProxy") {
       AddProxy(PROXY_HTTPS_HOST, value);
     } else if (key == "ftpProxy") {
       AddProxy(PROXY_FTP_HOST, value);
     } else if (key == "socksProxy") {
       // Older versions of KDE configure SOCKS in a weird way involving
       // LD_PRELOAD and a library that intercepts network calls to SOCKSify
       // them. We don't support it. KDE 4.8 added a proper SOCKS setting.
       AddProxy(PROXY_SOCKS_HOST, value);
     } else if (key == "ReversedException") {
       // We count "true" or any nonzero number as true, otherwise false.
-      // Note that if the value is not actually numeric StringToInt()
-      // will return 0, which we count as false.
-      int int_value;
-      base::StringToInt(value, &int_value);
+      // A failure parsing the integer will interpret it as 0 (false).
+      int int_value = StringToIntOrDefault(value, 0);
       reversed_bypass_list_ = (value == "true" || int_value);

[eroman, 2017/06/29 21:11:18]

(Some of this parsing is weird... but I am just trying to address the specific
use of StringToInt() in this CL, so meh).

[mmenke, 2017/06/29 22:21:20]

Could you are least change this to:

reversed_bypass_list_ = (value == "true" || StringToIntOrDefault(value, 0) !=
0);

"bool || int" is just weird, and brings back traumatizing memories of writing
Javascript.

[eroman, 2017/06/29 22:33:58]

Done.

     } else if (key == "NoProxyFor") {
       AddHostList(PROXY_IGNORE_HOSTS, value);
     } else if (key == "AuthMode") {
       // Check for authentication, just so we can warn.
-      int mode;
-      base::StringToInt(value, &mode);
+      int mode = StringToIntOrDefault(value, 0);
       if (mode) {
         // ProxyConfig does not support authentication parameters, but
         // Chrome will prompt for the password later. So we ignore this.
         LOG(WARNING) <<
             "Proxy authentication parameters ignored, see bug 16709";
       }
     }
   }
 
   void ResolveIndirect(StringSetting key) {
@@ skipping 625 matching lines @@
 void ProxyConfigServiceLinux::RemoveObserver(Observer* observer) {
   delegate_->RemoveObserver(observer);
 }
 
 ProxyConfigService::ConfigAvailability
     ProxyConfigServiceLinux::GetLatestProxyConfig(ProxyConfig* config) {
   return delegate_->GetLatestProxyConfig(config);
 }
 
 }  // namespace net