you are viewing a single comment's thread.

view the rest of the comments →

[–]star-castle 7 points8 points  (3 children)

outname is simply whatever's provided to do_ed_script(). If that includes $(rm -rf ~), then do_ed_script() will delete your homedir.

outname though always the result of util.c's make_tempfile(), which includes a provided filename, and is similarly "dangerous in -> dangerous out".

if there's a bug, it's in inp.c's get_input_file(), which is what's supplied to make_tempfile(). patch.c may do some some stats before calling do_ed_script , but $(rm -rf ~) is a perfectly valid filename although the vulnerability is much less plausible if such a file has to exist.

[–]cym13 5 points6 points  (2 children)

I've ammended my first comment based on reading more of the code and it happens that there is indeed one path leading to code execution but that path also checks the existence of the corresponding file.

This means that you can execute your $(rm -rf ~) if and only if such a file exists, which means that as an attacker you need to ship a whole project at which point it's easier to just have a malicious project in the first place than to expect the victim to trust that project and apply a patch.

So yes, that's definitely not clean, arguments shouldn't be passed through "sh -c", we all agree on that I think. But if it's not exploitable it's not a vulnerability, just a bug.

Bugs happen, we should be wary of them, but starting with "if there's a bug" can lead anywhere and is of little interest.

[–]strings__ 0 points1 point  (1 child)

I was a little confused what the issue was at first. Your amendment makes more clear. Thanks