Previewing merge directive meta-data contents is not possible

[Janne Snabb]

If I receive a merge directive (created by bzr send), there is no way to fully preview its contents including meta-data (such as full commit messages, committers and authors) before actually merging & committing it.

The only way that I figured out is as follows:

1. branch my main development trunk in to a separate merge directive merging branch

2. merge the received merge directive to the newly created branch

3. commit the merge to the branch

4. do "bzr log -n0" and similar things to see what actually came with the merge directive

5. merge the change to trunk if it is acceptable

The above workflow seems a bit tedious. There must be a simpler way to review merge directives. One alternative to the above is to merge and commit to trunk and then uncommit and revert if the received merge directive is not something I want... but I do not want to commit something to trunk before I know what it is even though uncommit is possible.

If I do not commit but just merge, I can do bzr diff and bzr st to see some data about the introduced changes, but I can not review the full commit messages for example (I can see only a small snippet of meta-data with bzr st).

I think I should be able to see the full contents of a merge directive INCLUDING the meta-data with "bzr merge --preview /path/to/merge-directive" and possibly with "bzr merge --interactive /path/to/merge-directive".

"bzr log /path/to/merge-directive" should also work.

I see this as a security vulnerability as well.

Let's think about the following scenario:

1. A project uses primarily merge directives to receive code patches.

2. I want to sabotage the project.

3. I look at their code until I find a genuine bug, a place where documentation could be improved, a typo or anything else worth a patch. This is easy with any open source project.

4. I make a patch.

5. I add some malicious content (see below) in the commit message or other meta-data.

6. I send a merge directive to the project.

7. The project reviews the merge directive with the current workflow. This means that only my patch gets reviewed but not the meta-data embedded in the merge directive.

8. My merge directive gets merged (with the malicious payload in the meta-data) because the patch is great.

9a. If the project is windows centric, suddenly many project developers start getting false anti-virus alerts when they pull my malicious meta-data which in this case happens to be a string which is identical to some virus pattern detected by many anti-virus products. The developers are unable to work with their branches any longer unless they disable their anti-virus software.

9b. If the project has loggerhead or other web interface to their Bazaar repository, I could embed some string which looks like malicious web site content when indexed by Google and others. Suddenly the project web site is flagged as hosting malware in Google search results and many "safe browsing" solutions.

10. It is troublesome for the project to recover from this situation because Bazaar does not offer a facility for editing (meta-data) history.

[Janne Snabb (snabb)]

There is a thread about this at bazaar mailing list:
http://comments.gmane.org/gmane.comp.version-control.bazaar-ng.general/70768

[Alexander Belchenko (bialix)]

Well, some comments about the attack you have described. Bazaar has no CLI-way to explore arbitrary revision metadata even for regular branches, not only for merge directives. So if somebody want to harm the project in the way you have described then it's possible to do even today with regular branches. Maybe a bit harder, e.g. you need Linux-based gatekeeper to full around WIndows anti-virus, but anyway it's possible.

[Vincent Ladeuil (vila)]

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.

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

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.

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.

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.

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.

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.

[Alexander Belchenko (bialix)]

Vincent Ladeuil пишет:
> 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.
>
> 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).

Recently I've got some criticism from one hg fanboy who see the absence
of visible metadata in the merge-directive as serious problem to
patches-based code-review workflow.

Vincent, you should remember those days when all bzr development went
through Bundle Buggy and the main way to get your patch reviewed and
approved was send it to bzr ML, while Bundle Buggy did the excellent job
of decoding and exposing those merge directives.

> bzr supports ftp, sftp, http, webdav, etc, so finding a server for a
> real branch seem to have been enough for now.

This is not the best answer :-/

--
All the dude wanted was his rug back

[Janne Snabb (snabb)]

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 s...

[Janne Snabb (snabb)]

Sorry but I'll bother you with one additional comment and try to shut up.

On Fri, 28 Jan 2011, Alexander Belchenko wrote:

> Bazaar has no CLI-way to explore arbitrary revision metadata even for regular branches, not only
> for merge directives. So if somebody want to harm the project in the way you have described then
> it's possible to do even today with regular branches.

Looks that way... In my opinion there should be an easy way to fully audit whatever is in the repository and whatever goes in. All bits of it, not just some part of it.

I have spent 10 years of my life watching over a sensitive CVS repository to make sure that nothing bad happens to it. My work uniform was a tinfoil hat and I still have the same mentality as you can see :). CVS repo is easy to audit. Bazaar repo is not. We probably could have not adopted Bazaar in that environment because of this reason.

[Martin Pool (mbp)]

This is definitely a real bug. Thanks for reporting it.

Rather than debating the situations where it would be a legal problem,
or the bug metadata, let's work out what a good user interface for
this should be. That's the best way to get it done and it may make it
easy to finish.

To me it seems the options are:

 * Make 'bzr log MP' show you all the revisions: that really ought to
work anyhow, regardless. (Maybe it does?)
 * Make the text form of the mp show all the metadata and comments.
(Nice for some users; not so much for others who just download it as a
file.)
 * Make merge show them as they're applied?
 * Make 'status -v' show all the pending merges?
 * Something else?

[Colin D Bennett (colinb)]

The whole "oh, no, some hidden stuff that looks like a virus signature exists got put on disk, even though it can't be executed or actually cause any harm" is honestly not Bazaar's problem. If your antivirus tool refuses to let you work (i.e., you can't tell it to ignore the content of Bazaar pack files, which make sense to omit from a virus scan since obviously these files are not executable in the repository itself) then you need to get a better tool.

This is really not related to merge directives anyway since the same situation could occur by merging a remote branch.

Back on topic: as to treating merge directives as branches for "bzr log feature1.patch" or "bzr st -r submit: feature1.patch", etc., I say +100!! I understand it's probably a lot of internal work to get this going for Bazaar, but as a user this would be extremely convenient.

[Martin Pool (mbp)]

On 5 February 2011 07:45, Colin D Bennett <email address hidden> wrote:
> Back on topic: as to treating merge directives as branches for "bzr log
> feature1.patch" or "bzr st -r submit: feature1.patch", etc., I say
> +100!!  I understand it's probably a lot of internal work to get this
> going for Bazaar, but as a user this would be extremely convenient.

It shouldn't be; internally they're treated as branch-like things. If
someone wants to have a go at exposing this in the ui it should be
feasible.