PSA: Did you know that it’s **unsafe** to put code diffs into your commit messages?

[cyrevolt@mastodon.social]

@zekjur gotcha - or rather: gitcha!

[musicmatze@social.linux.pizza]

@zekjur FWIW I reported this to the git ML... lets see

[ratsnakegames@mastodon.social]

@bmarinov @zekjur @max the behaviour of `patch` is intentional but the behaviour of the system as a whole is not.

[oldherl@comfy.social]

@zekjur@mas.to Wow, great example of the beautiful abstract layer provided by Modern Frontend™ broken by underlying email-based infrastructure.

[codecat@meow.social]

@zekjur i wonder if this works when the diff is hidden within a markdown comment <!— like this —>

That could be an extremely bad vulnerability

[funbaker@chaos.social]

@codecat @zekjur this is a HTML comment.

[romeu@mastodon.social]

@zekjur 😱😱😱😱😱

[lynn@a.bloodyno.se]

@zekjur genuinely why would this ever be the case in the modern day

[manawyrm@chaos.social]

@zekjur Ohh, that's nasty :D

[codecat@meow.social]

@funbaker yes those work in markdown

[halcy@icosahedron.website]

@zekjur oh, that's hilarious

[mo@mastodon.ml]

@funbaker ...and markdown is usually superset of HTML

@codecat @zekjur

[funbaker@chaos.social]

@mo @codecat @zekjur it isn't, but it can afaik inline HTML

[old_angry_queer@girlcock.club]

@zekjur well there ya go. You put the entire code of your malicious repo in the commit message and just put decoy code as the repo src.

[pq1r@tech.lgbt]

@zekjur classic lack of separation of in-band vs. out-of-band. Very similar to many security vulnerabilities get created, including some of the recent gpg.fail vulnerabilities.

[meph@social.treehouse.systems]

@zekjur weird little footguns

[sowophie@chaos.social]

@zekjur For being a central component of software development, "unified context diff" files are surprisingly fragile.

- Tools are supposed to skip "garbage" lines and apply everything that looks like a diff (this is what's happening here).

- Creating and deleting files is a done via implementation-specific hacks. GNU patch for example cannot create or delete an empty file.

- The format of file names is standardized, but the definition is lacking (see above), so Git does something else.

etc.

[setcab@rollenspiel.social]

@zekjur Why would someone design it like that? What's the possible use case for this behavior?

[randamumaki@mstdn.social]

@zekjur Just fucking stop using github.

[curiousicae@tech.lgbt]

@max
A strong case can be made though that the patch tool garbage erasure must only apply to initial “garbage”, rather than initial and inter-chunk garbage. That’s the aspect that causes the surprise here: VCS diffs only ever place commit metadata (including the message) before the actual patch, but the patch tool allows this metadata (“garbage”) anywhere. At least for git apply that is an obvious bug in and of itself.

Secondly, given this git show should have exported the commit message with one leading space on every line, as that is the escaping mechanism specified by patch for “garbage” data.

Doing both of the above changes would make for a robust mitigation on all levels.
@bmarinov @zekjur @musicmatze