use of temporary directory can lead to leak-ins

Bug #1742810 reported by Nish Aravamudan on 2018-01-11
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
usd-importer
High
Nish Aravamudan

Bug Description

Not sure of the right terminology, but since we use the equivalent of mktemp -d without a template, if there are already, e.g., tarballs in /tmp, then they will get used inadvertently (and not be checked).

It seems like the right approach is something different than we have now, but I'm not sure what yet.

Nish Aravamudan (nacc) on 2018-01-11
Changed in usd-importer:
status: New → Confirmed
importance: Undecided → High
assignee: nobody → Nish Aravamudan (nacc)
milestone: none → future

If context of the execution dir is breaking us we would want/need fully context depleted environments - like containers. But that pretty fast gets pretty heavy as we already see on build.

I first thought for taballs and such alone which do ../ we could just make a two level tmp dirs.
But we have hit examples of /tmp being used directly (non relative).

I wonder if we should use mount namespace execution like [1].
I'd think of the following on entry
1. make a new empty tmp dir via mktemp
2. make that the /tmp via mount namespaces
3. actual execution

If we identify more dirs we can add them on that mechanism as well.
Should be more efficient than full system containers each time, especially for dependencies just staying as is as long as we don't touch andthing in PATH and ld path.

[1]: https://blog.famzah.net/2014/06/04/private-tmp-mount-per-process-in-linux/

On Thu, Jan 11, 2018 at 10:53 PM, ChristianEhrhardt
<email address hidden> wrote:
> If context of the execution dir is breaking us we would want/need fully
> context depleted environments - like containers. But that pretty fast
> gets pretty heavy as we already see on build.

Yeah, we don't quite need that. What we need is an 'empty' location.
The problem was that using an empty location directlly at /tmp, where
.. is used means that stuff in /tmp is seen.

So what we need to do is use `mktemp -d -t gitubuntu.XXXXXX` or so. I
have some patches that start doing this.

> I first thought for taballs and such alone which do ../ we could just make a two level tmp dirs.
> But we have hit examples of /tmp being used directly (non relative).

Well, the intention is to clean this up directly.

> I wonder if we should use mount namespace execution like [1].
> I'd think of the following on entry
> 1. make a new empty tmp dir via mktemp
> 2. make that the /tmp via mount namespaces
> 3. actual execution

The problem is we have to get stuff out of the namespace into the
'top' namespace. I don't think that's trivial to do? Especially when
both might be /tmp?

> If we identify more dirs we can add them on that mechanism as well.
> Should be more efficient than full system containers each time, especially for dependencies just staying as is as long as we don't touch andthing in PATH and ld path.
>
> [1]: https://blog.famzah.net/2014/06/04/private-tmp-mount-per-process-
> in-linux/

Let's see if I can get well-defined semantics going first without namespaces.

-Nish

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers