Re: [Tails-dev] [Bug-wget] Wget Sending Original IP !!

Delete this message

Reply to this message
Author: Austin English
Date:  
To: The Tails public development discussion list
Subject: Re: [Tails-dev] [Bug-wget] Wget Sending Original IP !!
Rebasing it was trivial (the conflict was on adding the test to the
Makefile). It looks like upstream has a bug (they don't actually run
the tests), but that's fixed in this patch.

The tests pass for me on unstable (requires hacking out a warning
about using gets() though), I don't have a non-tails wheezy system to
test on. Tails/wheezy fails some other tests, though this particular
one does pass.

On Tue, Sep 1, 2015 at 8:55 PM, Austin English <austinenglish@???> wrote:
> On Sep 1, 2015 12:17 PM, "Austin English" <austinenglish@???> wrote:
>>
>> On Tue, Sep 1, 2015 at 11:08 AM, intrigeri <intrigeri@???> wrote:
>> > Hi,
>> >
>> > Austin English wrote (31 Aug 2015 19:48:50 GMT) :
>> >> On Tue, Aug 18, 2015 at 5:03 AM, intrigeri <intrigeri@???> wrote:
>> >>> Could you please check:
>> >>>
>> >>> 1. if this is worth a CVE ID
>> >
>> > Ping on this part? Without a CVE, it'll be painful to track by the
>> > Debian security team.
>> >
>> >>> 2. if the proposed patchset applies on top of Debian Wheezy's wget
>> >
>> >> The patch has been applied upstream:
>> >>
>> >> http://git.savannah.gnu.org/cgit/wget.git/commit/?id=075d7556964f5a871a73c22ac4b69f5361295099
>> >
>> >> it does not apply cleanly to 1.16.3, conflicting on tests. The source
>> >> changes, however, apply without conflict, I've attached that diff.
>> >
>> > OK, that's a good start. Thanks for checking! I think that resolving
>> > the merge conflict will make Debian security team more at ease taking
>> > the patch. Does it seem doable?
>> >
>> > Cheers,
>> > --
>> > intrigeri
>> > _______________________________________________
>> > Tails-dev mailing list
>> > Tails-dev@???
>> > https://mailman.boum.org/listinfo/tails-dev
>> > To unsubscribe from this list, send an empty email to
>> > Tails-dev-unsubscribe@???.
>>
>> I didn't try and won't be able to until next week.
>>
>> --
>> -Austin
>
> Note: the tests weren't too long, so it probably wouldn't be too difficult.
> I will look into it next week if no one beats me to it (but given that it is
> a security issue, I'd be happy if someone did ;) ).




--
-Austin
diff -urN wget-1.13.4.orig/src/ftp.c wget-1.13.4/src/ftp.c
--- wget-1.13.4.orig/src/ftp.c    2011-09-13 03:05:12.000000000 -0500
+++ wget-1.13.4/src/ftp.c    2015-09-07 14:01:10.694727053 -0500
@@ -249,7 +249,6 @@
   char *tms;
   const char *tmrate;
   int cmd = con->cmd;
-  bool pasv_mode_open = false;
   wgint expected_bytes = 0;
   bool got_expected_bytes = false;
   bool rest_failed = false;
@@ -841,13 +840,19 @@
                           ? CONERROR : CONIMPOSSIBLE);
                 }


-              pasv_mode_open = true;  /* Flag to avoid accept port */
               if (!opt.server_response)
                 logputs (LOG_VERBOSE, _("done.    "));
-            } /* err==FTP_OK */
-        }
+            }
+          else
+            return err;


-      if (!pasv_mode_open)   /* Try to use a port command if PASV failed */
+          /*
+           * We do not want to fall back from PASSIVE mode to ACTIVE mode !
+           * The reason is the PORT command exposes the client's real IP address
+           * to the server. Bad for someone who relies on privacy via a ftp proxy.
+           */
+        }
+      else
         {
           err = ftp_do_port (csock, &local_sock);
           /* FTPRERR, WRITEFAILED, bindport (FTPSYSERR), HOSTERR,
@@ -1106,8 +1111,8 @@
     }


   /* If no transmission was required, then everything is OK.  */
-  if (!pasv_mode_open)  /* we are not using pasive mode so we need
-                              to accept */
+  if (!opt.ftp_pasv)  /* we are not using passive mode so we need
+                         to accept */
     {
       /* Wait for the server to connect to the address we're waiting
          at.  */
diff -urN wget-1.13.4.orig/tests/FTPServer.pm wget-1.13.4/tests/FTPServer.pm
--- wget-1.13.4.orig/tests/FTPServer.pm    2011-01-01 06:12:35.000000000 -0600
+++ wget-1.13.4/tests/FTPServer.pm    2015-09-07 14:01:10.694727053 -0500
@@ -633,6 +633,14 @@
                     last;
                 }


+                if (defined($self->{_server_behavior}{pasv_not_supported})
+                    && $cmd eq 'PASV')
+                {
+                    print {$conn->{socket}}
+                      "500 PASV not supported.\r\n";
+                    next;
+                }
+
                 # Run the command.
                 &{$command_table->{$cmd}} ($conn, $cmd, $rest);
             }
diff -urN wget-1.13.4.orig/tests/Makefile.am wget-1.13.4/tests/Makefile.am
--- wget-1.13.4.orig/tests/Makefile.am    2011-07-20 04:37:15.000000000 -0500
+++ wget-1.13.4/tests/Makefile.am    2015-09-07 15:00:36.864394282 -0500
@@ -82,6 +82,7 @@
              Test-ftp-iri-fallback.px \
              Test-ftp-iri-recursive.px \
              Test-ftp-iri-disabled.px \
+             Test-ftp-pasv-not-supported.px \
              Test-HTTP-Content-Disposition-1.px \
              Test-HTTP-Content-Disposition-2.px \
              Test-HTTP-Content-Disposition.px \
diff -urN wget-1.13.4.orig/tests/run-px wget-1.13.4/tests/run-px
--- wget-1.13.4.orig/tests/run-px    2011-07-20 04:37:15.000000000 -0500
+++ wget-1.13.4/tests/run-px    2015-09-07 15:13:54.125469084 -0500
@@ -35,6 +35,7 @@
     'Test-ftp-iri-fallback.px',
     'Test-ftp-iri-recursive.px',
     'Test-ftp-iri-disabled.px',
+    'Test-ftp-pasv-not-supported.px',
     'Test-HTTP-Content-Disposition-1.px',
     'Test-HTTP-Content-Disposition-2.px',
     'Test-HTTP-Content-Disposition.px',
diff -urN wget-1.13.4.orig/tests/Test-ftp-pasv-not-supported.px wget-1.13.4/tests/Test-ftp-pasv-not-supported.px
--- wget-1.13.4.orig/tests/Test-ftp-pasv-not-supported.px    1969-12-31 18:00:00.000000000 -0600
+++ wget-1.13.4/tests/Test-ftp-pasv-not-supported.px    2015-09-07 14:01:10.698727046 -0500
@@ -0,0 +1,60 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use FTPTest;
+
+# This test checks whether Wget *does not* fall back from passive mode to
+# active mode using a PORT command. Wget <= 1.16.3 made a fallback exposing
+# the client's real IP address to the remote FTP server.
+#
+# This behavior circumvents expected privacy when using a proxy / proxy network (e.g. Tor).
+#
+# Wget >= 1.16.4 does it right. This test checks it.
+
+###############################################################################
+
+# From bug report 10.08.2015 from tomtidaly@???
+my $afile = <<EOF;
+FTP PORT command code in v1.16.3?
+
+In the past it could be possible for a site over http connection to
+redirect wget to FPT using FTP PORT command so the site gets the real IP
+of the computer even when wget proxy command is in use I believe:
+https://lists.torproject.org/pipermail/tor-talk/2012-April/024040.html
+
+Is that code still present in wget v1.16.3? It was present in v1.13.4.
+EOF
+
+$afile =~ s/\n/\r\n/g;
+
+
+# code, msg, headers, content
+my %urls = (
+    '/afile.txt' => {
+        content => $afile,
+    },
+);
+
+my $cmdline = $WgetTest::WGETPATH . " -S ftp://localhost:{{port}}/afile.txt";
+
+my $expected_error_code = 8;
+
+my %expected_downloaded_files = (
+    'afile.txt' => {
+        content => $afile,
+    },
+);
+
+###############################################################################
+
+my $the_test = FTPTest->new (
+                             server_behavior => {pasv_not_supported => 1},
+                             input => \%urls,
+                             cmdline => $cmdline,
+                             errcode => $expected_error_code,
+                             output => \%expected_downloaded_files);
+exit !$the_test->run();
+
+# vim: et ts=4 sw=4