From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 5837 invoked from network); 2 Aug 2020 08:52:11 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 2 Aug 2020 08:52:11 -0000 Received: (qmail 3161 invoked by uid 550); 2 Aug 2020 08:52:05 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 3143 invoked from network); 2 Aug 2020 08:52:04 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1596358313; bh=Ug4pO/fwbztf2K1hn0c0Ft9T2DxVinbmkXVKKpleyUw=; h=X-UI-Sender-Class:Date:From:To:Subject:References:In-Reply-To; b=hbcS4YgJxOGLGKYaTCel9I+s5+Y2i0DZhRyAnr8QS0DGt+96uuwKpVgUNs2qsEsy8 Tyhr6esjLOp4WHh3GOljYzohiAZo2GI1/D2hZvnlwo3n6N8OT4x/l1GHeYk1Xrgpbp pEbCUYRzSOwGfthk0WyJwFTHeEgA4mxYOt1SJ6tQ= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Date: Sun, 2 Aug 2020 10:51:52 +0200 From: Markus Wichmann To: musl@lists.openwall.com Message-ID: <20200802085152.GE2076@voyager> References: <20200801214216.10452-1-ariadne@dereferenced.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200801214216.10452-1-ariadne@dereferenced.org> User-Agent: Mutt/1.9.4 (2018-02-28) X-Provags-ID: V03:K1:/vPc0uP2Gc4CAaeZx645fh9dtRChaZgcVUBTSiK3rnnt+kCFIRf 0MBBCO7kOXWrm5PnV00tPUFZ1XssB1TeygHTVmrdWpa8RSNw05DDk1/yo1fy+qOcbT8cScr n/GLIzm9M1h+i5mLBCN7MCU1nTPSfk3C9M/Pnsw8G7GOfgP7YpT9UmtpHk60T1vMPDpKcB6 KvsydX7vEgBVQ1Wzrdvcw== X-UI-Out-Filterresults: notjunk:1;V03:K0:6ajLC92QQ6I=:zfMFtJtuoQge4R8bUy+1SQ SiQAq2NyWoe8dgAgXcGVaodyelwg8u588bX1pXR5m0uXJzM0Anhk4iFO8fy76ozWqZ+A5rPnP 8g+mb5O463PryWiELmkIWXB4be5UJ+/WCIwpwiXJ5IUToJeyX2PqRxXTEX9wqfKjLrW0NLLpK Ev1cDyDTUkJJ8EBB2wXVqWOdL71ghxW0XYJMLGfTF0s7TwNykjKhl9e1dbypPy+hnfBMRV9EX dwD9EE+i8Hxkv/oRwRNrxGSdGlsGLkogxDPsNsTs5GY82k78kOFpMNM3IK22Z+n5BAEAifT88 gPGTkLRV01L4K3p9wu+kSjGI259f89gdo+125usAfbJzo6rAfVRjzG9nK5aJtoSiRwDOoDRbh 0+4pwzQdwRI5xaLgB8V4fq70jU810AxoMpcPXYrgEd+75Kwy1sWtYY4gTrfWUiMviSAbHeuyg fkR4SPYb4h3forR1j61CBYtrvRS6DV41cFNO+LhCTwSpR2e2ddbu1mXlNBEEwzMTkqqLCgA73 cvKpQmC7AyfmQZ5u2TRYbChbkVUY1ShghVfCRs1Gq3q9NOPlGbSrHSU1irEJcHVgarB/ONJcU 5GYbIZmI/ktEb0zvd6t41BtkcYBVBC+z4vOxykn0eA2BMCKGF37qIUUM6Dzw5TPu5o2yxjUDy NhfUDJIbxKelJFvWeNrCtmLNIKnf0Xe5uI2h3FT4B8A6igrTzmhBpdGHVkN4jiWm1SpzEGQD0 iMWc2WnP+OF02ZJ42BNRKUPgIC6MD3hevODPyr6v/SqrZXdwgTPoNa4peFnWqSFloKka254Zk +YFw9XhWKJJjtIpC72sRPEzFvWYkjLl/fz3CxtutsSIInMTPmf5tCzcYenOT1DfNtscjy1dup lD4rzn8ev/AJneh1wj685giGaoJ/7m+U/Tbmt1nyhuDhj8/k+aE3bFO31WyvaT+l7FdrIpNDx 5PN4suN8ZkGL9sEk7YNIV4OkUUFzboWNKWta6slVKa2nKXsSebvbpGleJJEKGUN19gydzTDyK AcVS957EHygcOg2av1S/zp+OH64AAgP4JetW6UpEOY3Me/j/3vyvbTbdd61QMTJP1tjtMIaix 7yDQtMavD31DfY5NhwfmLS5VRLrK1eC9/RBzcBZjDS5inP8Ae6WR+oaZ36JrWXVvLe5updsS1 cWqqpmhJiBl8F+oWQs83qKyKDG4RyIlD1rAUlGcFN+d61A4klX88AWcZaz8Hgm+UffSPT2YwH 4Q1MMjn2io6J9g85N Subject: Re: [musl] [PATCH v3] implement recallocarray(3) On Sat, Aug 01, 2020 at 03:42:16PM -0600, Ariadne Conill wrote: > +void *recallocarray(void *ptr, size_t om, size_t m, size_t n) > +{ > + void *newptr; > + size_t old_size = om * n, new_size; > + > + if (n && m > -1 / n) { > + errno = ENOMEM; > + return 0; > + } > + new_size = m * n; > + > + if (new_size <= old_size) { > + memset((char *) ptr + new_size, 0, old_size - new_size); > + } > + > + newptr = realloc(ptr, m * n); > + if (new_size > old_size) { > + memset((char *) ptr + old_size, 0, new_size - old_size); > + } > + > + return newptr; > +} Use after free detected. Once realloc() returns, if newptr is not 0 and ptr is not 0, then ptr is free (unless ptr happens to be equal to newptr). Therefore the memset following the realloc() is invalid and may constitute a use after free. The only case when it is valid is when ptr == newptr, so you may as well use newptr there instead. Except you never test if the allocation succeeded, so on failing allocation that would crash. Also, should realloc() decide to move a shrinking allocation, this code would leave the first part of the memory in address space twice, and once in a freed location, so clearing it is invalid, but an attacker may still be able to read sensitive data. I guess, for maximum performance, you would want a version of realloc() that clears the old storage before freeing it if it does end up moving. Except that is not part of the standard realloc() contract, and any extension function would not be conducive to interposing libraries like libefence. The most portable options would be to always allocate new storage, or to never even call realloc() when shrinking. Not the most performant things in the world, but then, these extensions are meant for sensitive data. Also, the manpage (I found this one: https://man.openbsd.org/recallocarray.3) specifies an EINVAL error return. So, putting it all together, maybe something like this? void* recallocarray(void *ptr, size_t om, size_t m, size_t n) { if (!n || m > -1 / n) { errno = EINVAL; return 0; } size_t new_size = m * n; /* if ptr is null, oldnmemb is ignored, says the manpage */ if (!ptr) om = 0; size_t old_size = om * n; if (new_size <= old_size) { memset((char*)ptr + new_size, 0, old_size - new_size); return ptr; } void *newptr = calloc(m, n); if (newptr && ptr) { memcpy(newptr, ptr, old_size); explicit_bzero(ptr, old_size); free(ptr); } return newptr; /* on failure, errno is already set from calloc(), right? */ } As I said, always allocating new storage is not nice, but is the most portable way I know how to be able to still manipulate the old storage after the allocation of the new. Anything further would require sinking hooks deeper into the allocator. Unless I missed something. Ciao, Markus