Quoting Daniel Stenberg <daniel_at_haxx.se>:

> Thanks, but...
>
> You could easily solve the problem differently.

I understand this, but regardless you provide the --user option on the command
line, and it gives away the details to anyone who looks at the process list. In
the interests of security even for people whipping up quick hacks I'd encourage
you to support this feature.

> I'm sorry, but I can't accept this patch as it is. Writing to data that
> argv[] points to is not portable. Several operating systems will crash on
> such behavior.

So, unfortunately some people will not be able to take advantage of this, and
like you say, there are other ways to get the username:password unto curl
without it being leaked. But while --user can be used on the commandline it
should make that usage secure for those platforms that support it.

> I might consider adding this kind of code if you really insist and provide
> the suitable test code that we can run in the configure script to detect if
> this is indeed possible or not on this particular platform. Then we can
> have
> something like #ifdef HAVE_ALLOW_WRITING_ARGV_DATA around that section
> (since
> we must assume that it doesn't work until proven otherwise).

Ok, here's the patch again, this time with a patch to configure.in that tries to
run a small program that writes to argv. I've tested this by changing argv[0]
to argv[1] to force the segfault on my Debian machine, and HAVE_WRITABLE_ARGV
gets set appropriately here.

As an aside, the AM_CONFIG_HEADER macro didn't recreate src/config.h for me, so
I had to manually copy it from lib/config.h before compiling. It might be a
problem with the version of auto* that I'm using, but I didn't investigate further.

Anyway, I hope you will consider using this patch.

(patch at http://anchor.net.au/~jaq/curl-7.10.2-hidepasswd-2.patch if it's
munged here)

*** /home/jaq/src/curl/curl-7.10.2.orig/configure.in Sat Nov 16 01:11:20 2002
--- /home/jaq/src/curl/curl-7.10.2/configure.in Tue Jan 14 17:32:01 2003
***************
*** 318,323 ****
--- 318,339 ----
  fi
  
  dnl **********************************************************************
+ dnl Check if the operating system allows programs to write to their own argv[]
+ dnl **********************************************************************
+
+ AC_MSG_CHECKING([if argv can be written to])
+ AC_TRY_RUN([
+ int main(int argc, char ** argv) {
+ argv[0][0] = 'c';
+ return 0;
+ }
+ ],
+ AC_DEFINE(HAVE_WRITABLE_ARGV, 1, [Define this symbol if your OS supports
changing the contents of argv])
+ AC_MSG_RESULT(yes),
+ AC_MSG_RESULT(no)
+ )
+
+ dnl **********************************************************************
  dnl Check for the presence of Kerberos4 libraries and headers
  dnl **********************************************************************
  

*** /home/jaq/src/curl/curl-7.10.2.orig/src/main.c Sat Nov 16 01:15:28 2002
--- /home/jaq/src/curl/curl-7.10.2/src/main.c Tue Jan 14 13:02:52 2003
***************
*** 1616,1621 ****
--- 1616,1633 ----
      case 'u':
        /* user:password */
        GetStr(&config->userpwd, nextarg);
+ #ifdef HAVE_WRITABLE_ARGV
+ /* now that GetStr has copied the contents of nextarg, wipe the next
+ * argument out so that the username:password isn't displayed in the
+ * system process list */
+ if (nextarg) {
+ size_t len;
+
+ len = strlen(nextarg);
+ while (len)
+ nextarg[--len] = ' ';
+ }
+ #endif
        break;
      case 'U':
        /* Proxy user:password */

-------------------------------------------------------
This SF.NET email is sponsored by: FREE SSL Guide from Thawte
are you planning your Web Server Security? Click here to get a FREE
Thawte SSL guide and find the answers to all your SSL security issues.
http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0026en
Received on 2003-01-14