Traffic Server
  1. Traffic Server
  2. TS-2867

traffic_shell uses predictable file names in public writable directories

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.2.2
    • Component/s: None
    • Labels:
      None

      Description

      Forwarded from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=749846, thus quoting the reporter (removed ATS 3.0 arguments):

      The binary `/usr/bin/traffic_shell` contains the following strings, which
      should be sufficient to explain the issue:

      /bin/sort /tmp/zonetab.tmp > /tmp/zonetab

      I didn't look at the code in depth, but there are at least two
      errors here:

      • Predictable filenames, allowing file truncation/removal.
      • Race-conditions accessing files.

      The code in question comes from:

      trafficserver-3.0.5/mgmt/tools/SysAPI.cc + ConfigAPI.cc

      git head is not affected as traffic_shell was removed there, however older including 3.0, 4.0 and 4.2 branches are vulnerable to this. I suggest that you assign a CVE ID to track this issue and fix this issue in all supported branches.

      Note, that 3.0 has more vulnerabilities if you decide to fix this issue in 3.0 as well.

        Activity

        Hide
        ASF subversion and git services added a comment -

        Commit a62056d4b10d34abb3d0c439501f139673f594b0 in trafficserver's branch refs/heads/4.2.x from Phil Sorber
        [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=a62056d ]

        TS-2867: Remove clock functionality from traffic_shell to address temporary file handling issues.

        Show
        ASF subversion and git services added a comment - Commit a62056d4b10d34abb3d0c439501f139673f594b0 in trafficserver's branch refs/heads/4.2.x from Phil Sorber [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=a62056d ] TS-2867 : Remove clock functionality from traffic_shell to address temporary file handling issues.
        Hide
        Arno Toell added a comment -

        I don't see much reason to use a configurable temp directory location, the FHS clearly requires /tmp to be available (http://www.pathname.com/fhs/pub/fhs-2.3.html#TMPTEMPORARYFILES) and writable. Moreover, some init systems such as systemd might even isolate the /tmp space from concurring applications. Creating anyonmous temporary files to avoid all the /tmp attacks ought to be good enough in my opinion.

        Show
        Arno Toell added a comment - I don't see much reason to use a configurable temp directory location, the FHS clearly requires /tmp to be available ( http://www.pathname.com/fhs/pub/fhs-2.3.html#TMPTEMPORARYFILES ) and writable. Moreover, some init systems such as systemd might even isolate the /tmp space from concurring applications. Creating anyonmous temporary files to avoid all the /tmp attacks ought to be good enough in my opinion.
        Hide
        Leif Hedstrom added a comment -

        Yeah, those first two lines are from the Docs section from the .pm, you could argue they might be bad examples, but clearly not CVE material . The one in LogObject.cc is from a regression test.

        If we are not using proxy.config.temp_dir, nuke it? Alternatively, do we need to file an RFE to actually use proxy.config.temp_dir instead of hardcoded /tmp ?

        Show
        Leif Hedstrom added a comment - Yeah, those first two lines are from the Docs section from the .pm, you could argue they might be bad examples, but clearly not CVE material . The one in LogObject.cc is from a regression test. If we are not using proxy.config.temp_dir, nuke it? Alternatively, do we need to file an RFE to actually use proxy.config.temp_dir instead of hardcoded /tmp ?
        Hide
        James Peach added a comment -

        A quick grep shows a few suspicious lines:

        lib/perl/lib/Apache/TS/Config/Records.pm:  my $r = new Apache::TS::Config::Records(file => "/tmp/records.config");
        lib/perl/lib/Apache/TS/Config/Records.pm:  $r->write(file => "/tmp/records.config.new");
        proxy/Main.cc:  ProfilerStart("/tmp/ts.prof");
        proxy/logging/LogObject.cc:    tmpdir = "/tmp";
        

        Coverity #1022101 and #1196468 are flagging insecure temp file usage due to undefined umask setting. Both of these look largely safe to me.

        Interestingly, we have proxy.config.temp_dir, but nothing actually uses that.

        Show
        James Peach added a comment - A quick grep shows a few suspicious lines: lib/perl/lib/Apache/TS/Config/Records.pm: my $r = new Apache::TS::Config::Records(file => "/tmp/records.config" ); lib/perl/lib/Apache/TS/Config/Records.pm: $r->write(file => "/tmp/records.config. new " ); proxy/Main.cc: ProfilerStart( "/tmp/ts.prof" ); proxy/logging/LogObject.cc: tmpdir = "/tmp" ; Coverity #1022101 and #1196468 are flagging insecure temp file usage due to undefined umask setting. Both of these look largely safe to me. Interestingly, we have proxy.config.temp_dir , but nothing actually uses that.
        Hide
        Leif Hedstrom added a comment -

        Note that the only supported version(s) which still has traffic_shell is the 4.2.x release branch. Assigning this to 4.2.2.

        Show
        Leif Hedstrom added a comment - Note that the only supported version(s) which still has traffic_shell is the 4.2.x release branch. Assigning this to 4.2.2.

          People

          • Assignee:
            Phil Sorber
            Reporter:
            Arno Toell
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development