Hi, TL;DR
DO NOT remove the mktemp (or try to make it non-random). It is there for good security reasons. The actual bug seems to be related to copying the $TMPFILE to etc/..., so it should probably be fixed by simply not doing that. <adithya.balaku...@toshiba-tsip.com> writes: > On Sat, 15 Feb 2025 14:18:11 +0200 Anton Zinoviev <an...@lml.bas.bg> wrote: >> On Thu, Feb 13, 2025 at 06:32:55AM +0000, adithya.balaku...@toshiba-tsip.com >> wrote: >> > >> > Thanks for the feedback. Based on the idea above, I have attached a >> > revised >> > patch. Please have a look at the patch and let me know if you have any >> > concerns. >> >> The solution seem ok. However, I might be overlooking something but it >> seems to me that the commands >> >> + filename=$(echo $f | sed 's/\..*//') >> + dest_filename=$filename.fixed >> >> do not generate an immutable file name. > > Hi, > > Right now all temp files are created as "tmpkbd.XXXXXX" (See [1]). My > idea was to retain the "tmpkbd" part and ignore the random part of the > name. Hence I used the sed command to split the name at the "." Do you > suggest to keep an entirely different (consistent) name while copying > the file? It strikes me that this is an example of Chesterton's Fence. In other words: Removing the intentional randomness at that point is destroying the point of using mktemp in the first place (security of files created in world writable directories), so unless you completely understand what's going on in the code, and know for certain that such measures are not needed, it is something that should be left in place until you do. Having briefly looked at the code (and not yet fully understood where the file is managing to get into the target directory, and so causing this bug), I get the feeling that there is probably no reason to retain the TMPFILE, because if one is interested in it's contents, one should be using one of the --save options to setupcons. I note that 'trap' is called to ensure clean-up of $tempfiles (but the thing being cleared up by that is the $TMPFILE in in /run or perhaps /tmp, so not the file causing this bug directly). Presumably something (which I didn't yet find) is copying $TEMPFILE into the target directory before it gets deleted by the trap. If there really is any reason to keep that copy of $TEMPFILE, then it could be copied to a fixed name from the random mktemp name at that point, but my suspicion is that there's probably no point in copying it in the first place, in which case it should be excluded from the things copied, or deleted after the copy perhaps. I note that the initial report offers a way to reproduce the bug. I tried that, and got this: =-=-=- root@nimble:~# rm -rf /tmp/foo root@nimble:~# setupcon --setup-dir /tmp/foo root@nimble:~# setupcon --setup-dir /tmp/foo root@nimble:~# setupcon --setup-dir /tmp/foo root@nimble:~# ls /tmp/foo/etc/console-setup cached_UTF-8_del.kmap =-=-=- so I failed to reproduce this -- what am I missing? That's with console-setup 1.221 running on bookworm Debian -- does this need to be run in D-I or some other odd context to see the bug? In the original example, as well as the tmpkbd.* files, there was a 'null' which also seems like something quite odd was going on there. Cheers, Phil. -- Philip Hands -- https://hands.com/~phil
signature.asc
Description: PGP signature
