possible security issues

[calestyo]

Hey.

I was reading through the code and while I'm probably no expert (never used lua before) I might have found some issues.

First, we must generally assume that pathnames may contain any characters other than 0x0 (well at least on any POSIX system).

https://github.com/mar04/chapters_for_mpv/blob/78a8513e417daef4aff1a8fb2bba20a804167a18/chapters.lua#L196-L202 Should be unproblematic, since you control the input and all first parameters where invoking command_exists() contain no special characters. But at least that's something to keep in mind.

https://github.com/mar04/chapters_for_mpv/blob/78a8513e417daef4aff1a8fb2bba20a804167a18/chapters.lua#L221-L228 I have basically no Windows knowledge and even less powershell knowledge (like in less than zero ^^).
Is "Resolve-Path -Path \"" .. path .. "\" save? I mean if it were executed in a POSIX shell’s eval one could easily escape from that.
I guess it is safe, because the whole string is given as one argument, but would powershell perhaps allow something like: "Resolve-Path -Path "foo"; rm -rf "echo"… | …?

https://github.com/mar04/chapters_for_mpv/blob/78a8513e417daef4aff1a8fb2bba20a804167a18/chapters.lua#L243-L257 As far as I understand, this adds path as one argument to command... so should be safe, too, if so (I think even in the perl case).

https://github.com/mar04/chapters_for_mpv/blob/78a8513e417daef4aff1a8fb2bba20a804167a18/chapters.lua#L322-L334 That one I think could be exploited.

If path was e.g. foo; rm -rf /everything you'd end up executing: sh -c "printf %s foo; rm -rf /everything | … Not so good.

And remember, one must assume that user just downloaded some videos/music, and doesn't look at their filenames.

Can't you just feed the path via stdin to the process, using mpv’s stdin_data for subprocess?