Comment 5 for bug 709084

Revision history for this message
Janne Snabb (snabb) wrote : Re: [Bug 709084] Re: Previewing merge directive meta-data contents is not possible

On Fri, 28 Jan 2011, Vincent Ladeuil wrote:

> I doubt you can trigger an anti-virus from data embedded in a bzr format
> without using bzr itself to get access to it, in which case you know
> it's text and not a virus.

It would still cause lots of confusion and lost hours amongst the
developers. Also, as it is AFAIK not possible to edit the meta-data
history (unless you are a Bazaar ninja and not just an ordinary
user) and push those edits to everyone else in such a way that the
original malicious meta-data gets overwritten, the payload will
stay forever in the repository and cause issues in future as well.

After that happens you need to add a note on your project's web
site: "If you plan on working with our code, you need to disable
your anti-virus." Is that reasonable?

Please note that this (at this point theoretical) fake virus attack
was intended as just an example what could happen. The main point
is that it is easy to inject any data into a project without the
project having a way for screening for such things before it is too
late.

Many other things can happen as well, I will give you another extreme
scenario:

Where I live criticizing the leadership of the country can land you
in a prison. In some other countries it can get you killed. Someone
injects a piece of "forbidden speak" in a commit message. I finish
my work for the day, push my brach and go to sleep. It goes live
on my project's web site because I only saw a small snippet of it
and that part looked fine. Next morning I get arrested. I wait 2
years in a prison. I appear in a court and try to explain to the
judge "someone else injected it into my Bazaar branch and I couldn't
possibly have seen it before it was too late and got published on
my web site". The judge is likely to respond "Who is this Bazaar
you are blaming for your crimes against our Dear Leader? You are
sentenced to lifetime of hard labour." Well, it is not exactly that
bad here, but some other countries are far worse than Cambodia.

Given enough time I could invent lots of different scenarios where
having undesired data in the repository can be at least embarassing
if not worse.

> Similarly, I fail to see how a virus could run from inside a bzr data
> file.

I have not suggested a code execution vulnerability. Why do you
make it sound as if I did?

> Also, if there is no way from the bzr CLI to expose such metada, why
> would it be exposed in a browser ? And if it needed to be exposed, it's
> the web engine responsibility to quote whatever content it want to
> display to avoid such problems.

Well, the commit message, committer and author ARE displayed fully
in the browser. And with the current merging interface only a small
snippet of that can be reviewed when merging & committing. After
that it is too late already (especially if you have a light-weight
checkout where your commit goes live on the main server immediately
without pushing separately).

> Now to come back to your initial problem which this bug should focus on
> IMHO, a merge directive is not a branch today for bzr, so if you want
> access to more data inside the merge directive, you can indeed turn it
> into a branch.

Ok. So people are not supposed to be able to review contents of
merge directives without turning them into branches?

> The only use cases we've heard about merge directive usage to share code
> among a project is when there is no alternative for very small projects
> (as in 2 people).
>
> bzr supports ftp, sftp, http, webdav, etc, so finding a server for a
> real branch seem to have been enough for now.

If merge directives are not suposed to be used, why that feature
exists? If it exists for historical reasons, why is it not marked
as obsolete in the documentation?

I came across this issue because I received an unsolicted merge
directive from someone who made a welcome contribution to my project.
I found out that I can not see what it contains. It was the first
merge directive I ever saw. I am a Bazaar newbie and thus looking
at this issue from outside of the box.

> That being said, if you're interested in working on merge directives to
> make them behave as real branches, your patches will be warmly welcome.

My apologies, but this is unlikely to happen. At this point my
contribution to Bazaar will be just this bug report.

> Also, if you want to bring more security in a merge-directive based
> workflow, you can still sign the revisions and sign the emails, but even
> if you don't, remember that whoever manage to inject anything suspicious
> in the project is identified, whoever merge such contributions is
> identified, that's the point of a VCS.

This is not exactly relevant to this bug.

For most projects this is not practical. There are many
drive-by-contributions which come from people never heard of before
and never heard of after.

Authentication tokens get stolen/lost.

People become crazy, want to revenge to their boss or whatever after
performing perfectly for years. This happens in software industry
most often when people are laid off.

Also it might not matter if the malicious person can be later
identified. The damage is already done.

> ** This bug is no longer flagged as a security vulnerability

I disagree. If someone can inject any data in my repository without
me having any control over it, it IS a security vulnerability.

--
Janne Snabb / EPIPE Communications
<email address hidden> - http://epipe.com/