From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040625 Description of problem: While performing FTP upload to remote server, the remote credentials are displayed in clear text. Version-Release number of selected component (if applicable): How reproducible: Always Steps to Reproduce: 1. Launch mc 2. [F9] - select "FTP link..." 3. Enter username:password@remotehost information 4. Select file to upload 5. View remote host credentials Actual Results: This was taken during an actual upload. The filename and credentials have been changed and are not valid. ################################### Copy ################################### # # # Source msjavx.html # # Target /#ftp:joeuser:mypassword@ftp.georgetoft.com/FTP/msjavx.html # # # # File [ ] 62% ETA 0:00.45 # # Bytes [ ] 81% 43.83 KB/s # # Count [ ] 67% # # # # [ Skip ] [ Abort ] # # # ############################################################################ Expected Results: The remote host password (and preferably the username) should NOT be displayed. Additional info:
Hi George, the following patch should help you to get rid of this issue. Jindrich
Created attachment 103581 [details] This patch prevents mc displaying user's ftp password when uploading. This patch prevents mc displaying user's ftp password when uploading.
Created attachment 103582 [details] This patch prevents mc displaying user's ftp password when uploading. This patch prevents mc displaying user's ftp password when uploading.
Created attachment 103826 [details] The patch tells mc not to display password in path in copy dialog, delete, error dialogs, etc. The patch is tested, please apply.
Could you also get a patch like this for the directory hotlist?
I improved the patch a little bit and consulted it with Jakub. Ok, let me check it.
Jindrich, could you propose these patches on the mc-devel at gnome dot org list? That's probably easier. I've already started a thread on the issue.
Please mark patches as obsolete when appropriate.
Please compare http://mail.gnome.org/archives/mc-devel/2004-August/msg00167.html .
strip_password_only() can write to const *char path in this implementation. It can be fixed: new_path = g_strdup_printf ("%.*s%s", at - col, path, at); Also, upstream supports username with @ in it, so this function has to be reworked to check against last @ before /.
Andrew, thanks for the workaround. Leonard, that's a good idea, I'll play with the patches a bit more and do the proposal.
The improved version of the .strippwd patch fixes showing passwords in hotlist and error messages in case chdir fails in subshell, etc. With Jakub's help I modified strip_password in the way that strip_password_only () is no more needed. Leonard, the version of strip_passwd () is different from fix mentioned in mc-devel. With Jakub we concluded that our implementation that doesn't modify the input path at all is a bit better.
Created attachment 104935 [details] the improved version of strippwd patch Note that it uses mbstrlen () instead of strlen ().
Memory leaks, look for label_string. Also it seems there is one more typo in util.c: s/Cavevat/Caveat/ BTW, can you consult us about use of functions from GPLed src/util.c in the LGPLed vfs/*.c ? Is it legal?
Hi Andrew, I see no memory leak in function add2hotlist_cmd () regarding label_string. name_trunc () prints to static string so there's no need to g_free(). There's also no problem with input_dialog (). label_string is not g_free()d because it's done so in remove_group () called by done_hotlist (). If I missed something, can you please specify where exactly is the leak? Well, the vfs/*.c is licensed under LGPL because it used gnome-vfs some time ago but as I had a discussion on the topic with Jakub, there's no legal problem to use LGPLed source from GPLed programs. The license may change back to GPL if all the authors agree, what is rather rare case...
Hi, Jindrich label_string is leaked if input_dialog() returns NULL or empty string. In last case this memory (g_strdup("")) is also leaked. About our legal issue: we have reverse case. Now LGPLed VFS sources use GPLed functions from src/util.c and Roland Illig points this as a problem.
Created attachment 105280 [details] add_new_entry_cmd() without memory leaks. quick_dialog() and so add_new_entry_cmd() returns malloc()ed title and url. BTW platform should be changed to All.
Jindrich, can we fix this quickly so it can be committed before the release of 4.6.1? TIA.
Please see http://www.pavelsh.pp.ru/wiki/doku.php?id=mc-bugs , "hide FTP password in various places" for references to existing patches.
Leonard, sure, I'll have look at it.
Created attachment 105847 [details] the strippwd patch with the fix of possible memleak So finally I had a time to fix the possible memory leak Andrew pointed out. Andrew, have you posted your add_new_entry_cmd() patch also to mc-devel?
Hi, The significant part of the patch was applied upstream and mc-4.6.1-0.9 shouldn't have problems with showing ftp passwords in URLs any more.