Patch runs ed, and ed can run anything

[Spivak]

I will admit that people consider it a little surprising and non-hackers will be very loudly booed if they rely on this behavior but ed finds it way into lots of utilities like this. It's a remnant of a bygone era where people wanted their tools to have this kind of power.

But the author is absolutely right, nowadays we should prefer red. Still a small amount of shame if you're applying untrusted patches.

[zokier]

The early computing era (I'd say until early '90s) was definitely had something special in the way so much relied on trust, both trusting the user and trusting the wider community. Today everything is so locked down, paranoid and anxiety inducing that computing has become increasingly stressful and unfun.

I'm also reminded by a RMS writing (I think it was letter of some sort) campaigning that denying computer lab users root access was oppressive. Bit sad that I can't actually find it now.

And of course the classic (if misused like I'm doing here) Franklin quote "Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety."

[kragen]

I don't think it's so much that people wanted patch to be able to run arbitrary code, but rather that writing your tools to be very flexible allows you to get the functionality you need with very little code. The first version of ctags was written as a shell script that invoked ed to do the heavy lifting, for example. It was a small fraction of the rewrite of ctags in C in later Unixes.

So that's how you end up with m4 being Turing-complete, being able to put shell commands in anything, that sort of thing.

[colechristensen]

>It's a remnant of a bygone era where people wanted their tools to have this kind of power.

You should still want your tools to have this power.

It's amazing what you can do with tools built like this and how _easy_ it is. People use the same motivation for making microservices, "do one thing well", they just don't think about it for command line utilities and treat shell scripting like a second class citizen.

[gbrown_]

OpenBSD's patch handles ed style diffs internally now and also it is not permitted to execute other programs thanks to pledge.

https://marc.info/?l=openbsd-cvs&m=144498099601083&w=2

[seanhunter]

Patch knows that ed is a fine choice. Ed is generous enough to flag errors, yet prudent enough not to overwhelm the novice with verbosity.

In case anyone has not been exposed to the reference, it's perhaps invoking the famous `ed, man !man ed` page. https://www.gnu.org/fun/jokes/ed-msg.html

[rachelbythebay]

Crap! I knew I forgot something. I forgot to reference "eat flaming death". Oh well, good catch!

[knodi123]

geez, towards the end that starts reading like my bottle of Dr Bronner's Soap

[jimrandomh]

This is bad and should be fixed, but there are fairly few circumstances where it actually creates a new vulnerability. The majority of uses of patch are applied to source code by someone who's going to end up running that code anyways, so applying patches you haven't read closely from sources you don't trust is already unsafe.

[tzs]

> The majority of uses of patch are applied to source code by someone who's going to end up running that code anyways, so applying patches you haven't read closely from sources you don't trust is already unsafe.

I think you are overlooking a case that I suspect is common: applying a patch without reading the patch source and then using your source code control system to review what the patch did.

That lets you review that patch using whatever tools you normally use for reviewing code changes, which are often much nicer than reading a raw ed script.

[archgoon]

The main place where this would be an issue would be if you have a server that is not going to build or run code, but applies patches to source and outputs the result.

So a cr review website could be affected.

[tomc1985]

That advice has been being passed around for a good long time, probably almost as long as patch itself.

[IshKebab]

Sure, except you could (for example) steal Linus's private ssh keys with this.

[simlevesque]

You could build the binary on your machine and run it inside a VM.

[secure]

I don’t have ed installed (not a conscious decision), which prevents this from working:

% patch<evil.patch sh: 1: ed: not found patch: ed FAILED

patch works just fine for me, though, so ed is not required.

[ben_bai]

Patch applies files created with diff. Diff has 4 different output formats. One of those formats is basically a batch script with ed instructions.

For patch to work correctly you have to allow executing ed commands (internally or by spawning ed) but for security reasons you better not let ed execute yet another program.

[cbr]

What distribution?

(I don't think I've ever used a system that didn't have ed installed by default.)

[tantalor]

So patch can read email?

https://en.wikipedia.org/wiki/Zawinski%27s_Law

[dfsegoat]

Off topic, and forgive my naivety & ignorance, but I really do not understand the highly specific nature of this Principle / Law, as it is on wiki:

"Every program attempts to expand until it can read mail. Those programs which cannot so expand are replaced by ones which can."

... was Mail just the example at the time? and this is basically just the a generic reference to feature/scope creep?... I just don't get the highly specific inclusion of "Reading mail" as something that all programs expand towards.

[Blackthorn]

Eep. Seems like patch should be running red, not ed!

Amazing how an ancient vuln can still be found hidden in plain sight.

[oblio]

Does anyone know if the GNU tools, the coreutils, have been through security audits and fuzzing? They’re the most used tools on the planet, I’d say, and relying on the 90’s “many eyes make all bugs shallow” doesn’t seem to cut it anymore...

[emaste]

FreeBSD's patch runs red, a change we picked up from DragonFly BSD. That said OpenBSD's change to internally handle ed-style diffs is the best approach and I expect FreeBSD will migrate to that.

[volkadav]

similarly, i recall once being denied root on a host while trying to help out with something and then asking if i could at least have sudo less to look at things... which was granted. >:D (then i fixed the problem)

[jacquesm]

Always nice to know more about the system than those nominally in charge of the system.

[aplorbust]

"patch will attempt to determine the type of the diff listing, unless over-ruled by a -c, -e, -n, or -u option.Context diffs (old-style, new- style, and unified) and normal diffs are applied directly by the patch program itself, whereas ed diffs are simply fed to the ed(1) editor via a pipe."

According to this, context diffs are not sent to ed.

Is the author suggesting that patch can be fooled to interpret a context diff as an ed diff?

Theres a file called pch.c with an excessive amount of parsing and "intuit" functions like intuit_diff_type().

patch has anthromorphised progress and error messages and tries to "guess".

However I am only a dumb end user. I should not question what I do not understand. Its all safe I'm sure.

[gtrubetskoy]

Does this mean a "git pull" is vulnerable to this, or does git not rely on patch?

[gh02t]

I believe git uses its own implementation for applying deltas sucked down by `git pull`

    https://github.com/git/git/blob/master/patch-delta.c

There's also `git apply` that applies user supplied patchfiles but if I'm reading this correctly it also uses an internal implementation:

    https://github.com/git/git/blob/master/apply.c

[Jasper_]

git's delta pack format which used on the wire is not based on patch. This would only affect "git patch" and derivatives like "git am".

[jwilk]

The beep bug on HN: https://news.ycombinator.com/item?id=16753013

[teeray]

So how long will it be before we have a Monero miner implementation in ed?

[eridius]

Why does patch call ed? What did anyone ever actually use this for?

[LukeShu]

The `patch` program is expected to be able to apply any patchfile syntaxes created by `diff`. Once upon a time, the default behavior of diff was to spit out an `ed` script (nowadays, it's behind the `-e` flag). So, to apply that syntax of patchfile, it invokes `ed`.

[schoen]

Edit: I'm replacing my somewhat vague reply with a more correct answer:

https://en.wikipedia.org/wiki/Diff_utility#Edit_script

[zokier]

Unix principle of small tools chained together.

[brynet]

Text editors are shells, remember that if your program executes a text editor.

[hannob]

Here's the upstream bug report: https://savannah.gnu.org/bugs/index.php?53566

[zitterbewegung]

This hole needs a fancy name. Patchowned? Pwneded ?

[rachelbythebay]

I was hoping "bangpatch" (see URL). It's descriptive, and it's also a sly reference to the old-school days of UUCP.

[gruez]

PatchEd

[paulie_a]

No, just no. Naming vulns is a theme that needs to go away

[gaius]

The Patch Snatch Batch

[yiyus]

This is, of course, a vulnerability. But let's not get too crazy. Once you are applying a patch, either you trust the patch or not.

If you blindly apply patches, you will be in risk as soon as you run or try to compile the patched command. This attack is just a bit faster because it happens as soon as you apply the patch.

[mrob]

Patch isn't only used for source code. I wouldn't have expected any risk of malicious code execution if I was patching documentation.

[schoen]

Suppose you're applying patches to something other than software?

[0x0]

What if you're trying to apply a patch to a .txt document?

[cs702]

My reaction whenever I read things like the OP: https://blog.codinghorror.com/assets/images/codinghorror-app...

[vilhelm_s]

The whole (satirical) website https://holeybeep.ninja/ is really great.