]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Fixed "Invalid read of size 1" squid.conf parsing error in r14719.
authorAlex Rousskov <rousskov@measurement-factory.com>
Wed, 21 Sep 2016 16:26:39 +0000 (10:26 -0600)
committerAlex Rousskov <rousskov@measurement-factory.com>
Wed, 21 Sep 2016 16:26:39 +0000 (10:26 -0600)
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.

src/cf_gen.cc

index fcacff0d5f297b5f3e6a710c3ba7abf7165f60e2..3dea7a96dec7467780c214f830c1452d0ea37738 100644 (file)
@@ -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 <<