From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id C3B9B5CD87; Thu, 21 Jan 2021 05:18:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C3B9B5CD87 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1611195494; bh=e13F0YY+2x8CJUpo3Afth9w2z8kP/mlEXMUTwdV1VPQ=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=pUpaqOGSBcWxLkMjFY5QPqN8KOjCwluI/P73fGB3p9ldPW0Ny9w91BmR/xWwORpKZ 7GYUjnocVHeahYMFlGu4bfBsxrO0+aGJVpvr/1xyqhRuvPm3Amnz6MkVYQw8FhW2AS PiumpZ1ycI5v5SBn1V2+IISP76GtZ1LEUR4fJfpY= Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 410DB645E6 for ; Thu, 21 Jan 2021 05:18:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 410DB645E6 Received: by smtp54.i.mail.ru with esmtpa (envelope-from ) id 1l2PY4-0007Kd-4D; Thu, 21 Jan 2021 05:18:12 +0300 Date: Thu, 21 Jan 2021 05:16:27 +0300 To: Sergey Nikiforov Message-ID: <20210121021627.2dzyh7fho2cvjlrz@tkn_work_nb> References: <7f5ce3321cda6d4305282af486ed2f493b920d1f.1610357625.git.void@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <7f5ce3321cda6d4305282af486ed2f493b920d1f.1610357625.git.void@tarantool.org> X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D0E79FBC973162CD520EA9A9E182CF906B1BCD0E5756442500894C459B0CD1B92D08A5F3B4ACBF4D15B164F395C53BBB7EA65E2FD674F6C9A65EC38BD4A67DDF X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE78524CA6C6209E9ADEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375C4806A08D329A618638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FCD2D52E5455765B93DEB13DB6E896E3DAF839EFD8F31A5789389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0A3E989B1926288338941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B68424CA1AAF98A6958941B15DA834481F9449624AB7ADAF37BA3038C0950A5D3613377AFFFEAFD269176DF2183F8FC7C0EA93887B71B66F2BD8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE34D3DDB508812D900BA3038C0950A5D36B5C8C57E37DE458B92C30CA92C8B5C6467F23339F89546C55F5C1EE8F4F765FC9C5AF73B5E338C8A75ECD9A6C639B01BBD4B6F7A4D31EC0BC0CAF46E325F83A522CA9DD8327EE4930A3850AC1BE2E735843AE0F20224B8D0C4224003CC836476C0CAF46E325F83A50BF2EBBBDD9D6B0F03FC5E6FBEB4333C3B503F486389A921A5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5A3581C0DB3A77650E2FC6915EAE496D45F4C5E81065B891BD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA758F9E841AEAEC4F2C410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D343FB425EC7F4D4A4B23C8E86D73685A5DFBE261951B3756079DF76B47487D588E8F0A7734E1A624571D7E09C32AA3244C84DE10C4B9B09F10FB512EF621B28FEA725D5B54B2FE4575927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj7AvRt3Uvx5QMADp5b7g6Ig== X-Mailru-Sender: FFAA8E4AEE17E37C3731A083A1A85ADE2D08A5F3B4ACBF4D15B164F395C53BBBB7EA9FE7735C3DBFC664A44C781FCEA7C77752E0C033A69EDF9F2CE1E9CF805D8CD356D4F938FF726C18EFA0BB12DBB0 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v6 1/2] base64: fix decoder output buffer overrun (reads) X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Alexander Turenko via Tarantool-patches Reply-To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Mon, Jan 11, 2021 at 12:45:00PM +0300, Sergey Nikiforov wrote: > Was caught by base64 test with enabled ASAN. It seems, we have a problem in CI, otherwise it would be detected. At least, I don't see an explicit suppression relevant to the base64 code or disabling the test under this CI job. Can you, please, investigate, how to enable it in CI, so we'll catch the similar problem next time if it'll appear? > > It also caused data corruption - garbage instead of "extra bits" was > saved into state->result if there was no space in output buffer. We have the dead code and it appears to be broken. Why don't remove it? (AFAIS, the rest of the code does not read off the buffer.) Is it due to a little probability that we'll decide to support chunked decoding and we'll decide to implement it in exactly this way (and not just leaving undecoded bytes in, say, ibuf)? Another side of this little probability is: * The code complexity and so waste of the time for anyone who need to dive into it. * Have untested code in the code base that may give us more surprises. * Extra def-use dependencies may hide optimization opportunities and increase register pressure. This is not the question regarding the patch, but this code looks broken in several other ways. At least: * It skips unrecognized symbols and does not report a decoding error. * If the output buffer is too short, it neither report an error, nor a required buffer length (like snprintf()). No way to distinguish a successful and an 'interrupted' processing. > > Added test for "zero-sized output buffer" case. Nice catch. > > Fixes: #3069 > --- > > Branch: https://github.com/tarantool/tarantool/tree/void234/gh-3069-fix-base64-memory-overrun-v6 > Issue: https://github.com/tarantool/tarantool/issues/3069