Ticket #4535 (closed enhancement: fixed)

Opened 7 months ago

Last modified 8 weeks ago

Temporary directory name conflicts when runnig inside containers with shared /tmp

Reported by: eugenesan Owned by: andrew_b
Priority: major Milestone: 4.8.32
Component: mc-core Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

When running inside Distrobox container temporary directory name conflicts with mc instance running on the host machine which has different permissions:

root@ubuntu:~# mc
Directory /tmp/mc-root is not owned by you
Temporary files will be created in /tmp
Press any key to continue...

Trivial fix I came up with is to add process ID to the directory name.
Also, that might be useful when running multiple instances on the host.

Please find the fix attached to the tocket

Attachments

mc-tmp-pid.patch (1.3 KB) - added by eugenesan 7 months ago.
Add process ID to the temporary directory name

Change History

Changed 7 months ago by eugenesan

Add process ID to the temporary directory name

comment:1 Changed 7 months ago by andrew_b

  • Status changed from new to accepted
  • Owner set to andrew_b
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.32
Last edited 7 months ago by andrew_b (previous) (diff)

comment:2 follow-up: ↓ 3 Changed 7 months ago by zaytsev

Hi Andrew, I'm not sure what was the original reasoning behind making the directories per-user, but now that you're going into the direction of making them unique (per process), isn't it much better to use the POSIX mkdtemp API (or rather get_tmp_dir from glib) and remove all our strange black magic altogether? There was already CVE-2004-0231 in the past due to homegrown adventurous handling of temporary directories...

comment:3 in reply to: ↑ 2 Changed 7 months ago by andrew_b

Replying to zaytsev:

isn't it much better to use the POSIX mkdtemp API

Probably yes.

(or rather get_tmp_dir from glib)

g_get_tmp_dir uses $TMPDIR. mc tries to use $MC_TMPDIR if it's set and $TMPDIR otherwise.

comment:4 Changed 7 months ago by andrew_b

  • Branch state changed from on review to on rework

comment:5 follow-up: ↓ 6 Changed 7 months ago by zaytsev

g_get_tmp_dir uses $TMPDIR. mc tries to use $MC_TMPDIR if it's set and $TMPDIR otherwise.

Well, there seems to be another more flexible API: g_dir_make_tmp - maybe you could use that instead.

comment:6 in reply to: ↑ 5 Changed 7 months ago by andrew_b

Replying to zaytsev:

g_get_tmp_dir uses $TMPDIR. mc tries to use $MC_TMPDIR if it's set and $TMPDIR otherwise.

Well, there seems to be another more flexible API: g_dir_make_tmp - maybe you could use that instead.

It doesn't help us: g_dir_make_tmp calls g_get_tmp_dir.
I think we should keep the $MC_TMPDIR usage, so we have to write a code for temporary directory name. Something like

    g_snprintf (buffer, sizeof (buffer), "%s/mc-XXXXXX", sys_tmp);
    tmpdir = g_mkdtemp (buffer);
    if (tmpdir != NULL)
        g_setenv ("MC_TMPDIR", tmpdir, TRUE);

comment:7 Changed 7 months ago by zaytsev

You are right, I agree!

comment:8 Changed 7 months ago by andrew_b

comment:1 updated.

comment:9 Changed 7 months ago by andrew_b

  • Branch state changed from on rework to on review

comment:10 Changed 7 months ago by zaytsev

  • Votes for changeset set to zyv
  • Branch state changed from on review to approved

comment:11 Changed 7 months ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from zyv to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

comment:12 Changed 7 months ago by andrew_b

  • Status changed from testing to closed

A typo in the commit message: s/drirectory/directory/ :-(

comment:13 Changed 8 weeks ago by anton_kg

this commit broke /usr/libexec/mc/mc-wrapper.sh script probably. It expects /tmp/mc-$MC_USER
FYI.

Last edited 8 weeks ago by anton_kg (previous) (diff)

comment:14 Changed 8 weeks ago by zaytsev

This has been fixed in #4575. FYI.

Note: See TracTickets for help on using tickets.