From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id c1d0a270 for ; Sun, 21 Apr 2019 20:09:09 +0000 (UTC) Received: (qmail 11891 invoked by alias); 21 Apr 2019 20:08:52 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 44241 Received: (qmail 21427 invoked by uid 1010); 21 Apr 2019 20:08:52 -0000 X-Qmail-Scanner-Diagnostics: from mail-it1-f195.google.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.101.1/25426. spamassassin: 3.4.2. Clear:RC:0(209.85.166.195):SA:0(-2.2/5.0):. Processed in 1.194538 secs); 21 Apr 2019 20:08:52 -0000 X-Envelope-From: mikachu@gmail.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: pass (ns1.primenet.com.au: SPF record at _netblocks.google.com designates 209.85.166.195 as permitted sender) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=c3Mcgx+PpF+CkTHEKiJaOPSaF5KT7NhYKWgg/tye1lo=; b=CpCjlS5smOIOfGSVfMLe1giFRZD6Po8SUMRqLrVaJh7R2b6Zz62pf5rfF4l6YP6V9C NfXbQ+XikhAqY2LwBM/KsrE8sgCk83Po3OgZfEGxkfqZ83pKIucLEnpSC5C685PkLwq9 YQCBIbYhjA3BnzZhh4GQP3MnWl4d4y5qFBc/6+AW/ERXrEebOwFbpGTWUUGldBisP2Nn GuX9xnUCmu5CDkpXMJalK/dBXpU6fg2w6113NF3sBfXB3pegdgm6gg6W5yqiIt71BNYv 1aracaBec9TKLYR3u+v6b5jxKE5K1Q07c5pZTzlx0x3ejRKSwQ1SgDiuaYTKgQOykOlT cwxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=c3Mcgx+PpF+CkTHEKiJaOPSaF5KT7NhYKWgg/tye1lo=; b=CViFJQC+w2xUcEGbMhp3+JFjJ8i3CD4bbl1FObz0Dyh1dDZ5ZCBx5hQqMhDxGIC37y 3mzWZs7QqS4WNwbc/BPaudQA/BuUb361ZrYxXz7CHJDspzKMkR6jITfZPvqblAK0f43x Se2Y38WMVHVHlJBXAF/X40RYRkxOuZhpJ/XGOrgaLdDwmdcZRqdSPnhoouyGGN0SW95j KQPRkEjqlPINJk+J9hfni4V6bzKTN01K6IRDFl/cZ0oeBOwLNHlEbUK78c5+gO/BWLOS +6fHoR/wAifvWypKH/DqUJWRkCaWtDaBBmCdeFI3M/BlKXKv3LT2Y+NWsCC4UNMKer+A pUpQ== X-Gm-Message-State: APjAAAXlYFv/yNKpHcchDwoTU5MSCLRVfvLrXKqSnIM2qIqfRdNtDK6H vehVx/K8A6DjuJQthuCzANLxPbmIyHIs6AWGmGSNkQ== X-Google-Smtp-Source: APXvYqyBkkwa/sV8h3fogjHzzs/92lyUTSnIUit8DYfynyukEKN2oe6JAsRf/1QaoVqP4yyv1GKFWgVPN/j2+G+KJ4w= X-Received: by 2002:a24:7c9:: with SMTP id f192mr11256501itf.97.1555877296912; Sun, 21 Apr 2019 13:08:16 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <5315c85d-abda-fdb5-271f-22a7805eea27@zentaur.org> References: <20190420043349.GA52075@CptOrmolo.darkstar> <5315c85d-abda-fdb5-271f-22a7805eea27@zentaur.org> From: Mikael Magnusson Date: Sun, 21 Apr 2019 22:08:16 +0200 Message-ID: Subject: Re: [PATCH] replacement for mktemp and mkstemp code in Src/utils.c To: Clinton Bunch Cc: zsh-workers@zsh.org Content-Type: text/plain; charset="UTF-8" On 4/20/19, Clinton Bunch wrote: > Mikael Magnusson wrote: >> On 4/20/19, Matthew Martin wrote: >>> On Fri, Apr 19, 2019 at 02:54:47PM -0500, Clinton Bunch wrote: >>>> On at least one system mktemp produces very predictable names: >>>> >>>> % () { print File: $1; cat $1 } =(print "Hello World") >>>> File: /tmp/zsh010785 >>>> Hello World >>>> % () { print File: $1; cat $1 } =(print "Hello World") >>>> File: /tmp/zsh010785 >>>> Hello World >>>> % () { print File: $1; cat $1 } =(print "Hello World") >>>> File: /tmp/zsh010785 >>>> Hello World >>>> % echo $$ >>>> 10785 >>>> >>>> This provides an alternate implementation for generating and opening >>>> temp >>>> file names. I considered only using this implementation on known bad >>>> systems, but I have no way of knowing all of them (or testing for them >>>> in >>>> configure). I see no reason to expect system implementations of mktemp >>>> or >>>> mkstemp to be significantly faster than mine unless written in assembly >>>> (which seems unlikely). >>> I would strongly prefer using the implementation only on known bad >>> systems (or prodding the relevant vendors to fix their system). I don't >>> think speed should be the main consideration here; rather the primary >>> concern should be security. While your patch is certainly better than >>> using the native mktemp on at least one system, it would be worse than >>> the native mktemp on say FreeBSD which uses arc4random_uniform which >>> does not require a user provided seed nor does it have modulo bias. > We still face the problem of determining which systems have broken > implementations. I know of one, that doesn't mean there aren't others. > My implementation could easily be modified to use arc4random_uniform > on those system on which it is available if that's the primary > objection. I'd have used /dev/urandom (at least as a seed) if it were > available everywhere. >> The commit message is incomplete, it claims that mktemp is insecure on >> one system, therefore it replaces mkstemp, which makes no sense. Is >> mkstemp also insecure on that system, is it not available at all? Do >> we not even attempt to use mkstemp? >> >> Also, please make at least some attempt to use the same coding style >> as the rest of the code base, ie try to use the space bar sometimes. >> (You're not even consistent with yourself in some places). >> > mkstemp uses the same insecure naming structure. That is the normal > case so it seemed redundant to mention it. mkstemp returns an open file descriptor whereas mktemp returns a string. According to the specification using mkstemp will always create a new file that didn't exist, and is safe. If an OS implementation somehow clobbers existing files when you call mkstemp() then that's very broken and should be immediately fixed. The worst outcome of the name being predictable is that someone can perform a DOS attack against you, ie preventing you from creating a file, but they won't gain control of any created files. > Are the code style settings codified anywhere? The tabs were added by > Vim's autoident. A set of options for indent or astyle would make it > easier for anyone to meet the coding style. Yeah, Etc/zsh-development-guide. It doesn't explicitly mention that you should put spaces around operators, but you should do that too. Ie, these lines +ztempfile(char *template,int slen,enum zmt_action action, int *fd) + int padlen=0,r,i,len,success=0,failure=0,attempts=0; and others should look like this +ztempfile(char *template, int slen, enum zmt_action action, int *fd) + int r, i, len; + int padlen = 0, success = 0, failure = 0, attempts = 0; -- Mikael Magnusson