Fujii Masao
masao****@gmail*****
2015年 7月 17日 (金) 18:34:34 JST
On Fri, Jul 17, 2015 at 4:06 PM, Sawada Masahiko <sawad****@gmail*****> wrote: > On Thu, Jul 16, 2015 at 10:06 PM, Fujii Masao <masao****@gmail*****> wrote: >> On Thu, Jul 16, 2015 at 8:49 PM, Sawada Masahiko <sawad****@gmail*****> wrote: >>> Hi all, >>> >>> According to CoverityScan, ludia_funcs probably has security problem >>> around creating temporary file. >>> Returning wrong result by rewriting temporary file by attacker before >>> returning client is possible. >>> mkstemp(), is used currently ludia_funcs, creates temporary file with >>> 0600 permission already, but a such behavior might be changed at some >>> day. >>> Attached patch adds setting of umask before creating temporary file. >> >> Good catch! >> >> +#define TP_MKSTEMP_UMASK 0177 >> >> Isn't it better to move this near other textporter's macro variables? >> If yes, I think that it's better to use TEXTPORTER_ as the prefix of >> the variable name. > > I agree with you, latest patch attached. > >> + * For security reason, set umask to ensure creating temporary >> + * file with 0600 permission. >> >> Could you elaborate the "security reason" in the comment? Otherwise >> I'm afraid that we can easily forget what the "security reason" is here. > > If the file permission is not set properly, non-execution user who > doesn't have permission to access temporary file can read them, > which leads information leak, I think. I pushed the slightly-modified version of the patch. Thanks! Regards, -- Fujii Masao