From: Alex Rousskov Date: Wed, 21 Sep 2016 16:26:39 +0000 (-0600) Subject: Fixed "Invalid read of size 1" squid.conf parsing error in r14719. X-Git-Tag: SQUID_4_0_15~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c2a378768ba629f47919277177b7b35cfef6e69b;p=thirdparty%2Fsquid.git Fixed "Invalid read of size 1" squid.conf parsing error in r14719. Valgrind found an off-by-one ProcessMacros() length argument usage: Invalid read of size 1 by ReplaceSubstr(char*&, int&, ...) (cache_cf.cc:323) by SubstituteMacro(char*&, int&, ...) (cache_cf.cc:338) by ProcessMacros(char*&, int&) (cache_cf.cc:344) by default_line(char const*) (cf_parser.cci:13) by default_all() (cf_parser.cci:136) by parseConfigFile(char const*) (cache_cf.cc:581) ProcessMacros() assumes it is modifying a c-string and needs the number of characters in that string (i.e., length), not length+1. Needless to say, all this error-prone c-string manipulation code should be replaced! Also replaced potential out-of-bounds config_input_line writing code. XXX: Parsing should fail if config_input_line is not long enough. --- diff --git a/src/cf_gen.cc b/src/cf_gen.cc index fcacff0d5f..3dea7a96de 100644 --- a/src/cf_gen.cc +++ b/src/cf_gen.cc @@ -478,10 +478,10 @@ gen_default(const EntryList &head, std::ostream &fout) fout << "static void" << std::endl << "default_line(const char *s)" << std::endl << "{" << std::endl << - " int len = strlen(s) +1;" << std::endl << - " char *tmp_line = xstrndup(s, len);" << std::endl << + " char *tmp_line = xstrdup(s);" << std::endl << + " int len = strlen(tmp_line);" << std::endl << " ProcessMacros(tmp_line, len);" << std::endl << - " xstrncpy(config_input_line, tmp_line, len);" << std::endl << + " xstrncpy(config_input_line, tmp_line, sizeof(config_input_line));" << std::endl << " config_lineno++;" << std::endl << " parse_line(tmp_line);" << std::endl << " xfree(tmp_line);" << std::endl <<