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 7412958A4C9; Tue, 15 Aug 2023 17:38:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7412958A4C9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1692110283; bh=FkoH492IPtucyjVlJcfyiP74tsdp4uhCeLdDVrdT+6k=; 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=g3MQR1tRYR3zPCwPay6X1jB4s/PoSxIKVkvHAKp8PqA/ioeaS2PtCNSCMyQi0pT3i QHJPI09lI9v38WeH4kstAst3CFPER7SKOur4F8DDoGfq/FXO2+tyUHF0jBToFDV4tK XnzNMrPizMZRHvR4p4jhN00i0Gj7rYBB2dvLR4Is= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [95.163.41.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 80C5E58A4C9 for ; Tue, 15 Aug 2023 17:38:02 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 80C5E58A4C9 Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1qVvBJ-00BuHW-2o; Tue, 15 Aug 2023 17:38:02 +0300 Date: Tue, 15 Aug 2023 17:38:01 +0300 To: Sergey Kaplun Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD969E04B5EED670DC86EE92E42F0C271DDC310B0E58918727B182A05F538085040F0F6679718B9BEEC1EB62328B059413931575A8FD4E3F598964098A928E36EFC X-C1DE0DAB: 0D63561A33F958A5F7222EA5AE897551F1868BF267D905D9F4A407CEE9E0D4C6F87CCE6106E1FC07E67D4AC08A07B9B04E7D9683544204AFCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3498910055B812BD9CB2C3B5646AD820E4AD49FC3E629ECFBB8D56A475E597C1DC47A78254809577901D7E09C32AA3244C68B4711755DD5D7F1C43CA2CA1DDCFD0C3B3ADDA61883BB5BAD658CF5C8AB4025DA084F8E80FEBD396F07DFE06A4A8314E894E437E78228B66933FA05BD8EF0CAD958392AE682691 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojHVl7ekwB6hhYFhCpqIVBZQ== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE4074F51CDD029204C18D2A4D59AA0ABA52F8D65200875EB1684D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit 16/19] Prevent integer overflow while parsing long strings. 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the patch! LGTM, except for a few comments below. On Wed, Aug 09, 2023 at 06:36:05PM +0300, Sergey Kaplun via Tarantool-patches wrote: > From: Mike Pall > > (cherry-picked from commit 16e5605eec2e3882d709c6b123a644f6a8023945) > > This commit fixes possible integer overflow of the separator's length Typo: s/possible/a possible/ > counter during parsing long strings. It may lead to the fact, that > parser considers a string with unbalanced long brackets to be correct. Typo: s/parser/the parser/ > Since this is pointless to parse too long string separators in the hope, Typo: s/this is/it is/ > that the string is correct, just use hardcoded limit (2 ^ 25 is enough). Typo: s/use hardcoded/use the hardcoded/ > > Be aware that this limit is different for Lua 5.1. > > We can't check the string overflow itself without a really large file, > because the ERR_MEM error will be raised, due to the string buffer > reallocations during parsing. Keep such huge file in the repo is Typo: s/Keep such/Keeping such a/ > pointless, so just check that we don't parse long string after Typo: s/long string/long strings/ > aforementioned separator length. Typo: s/aforementioned/the aforementioned/ > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#8825 > --- > src/lj_lex.c | 2 +- > .../lj-812-too-long-string-separator.test.lua | 31 +++++++++++++++++++ > 2 files changed, 32 insertions(+), 1 deletion(-) > create mode 100644 test/tarantool-tests/lj-812-too-long-string-separator.test.lua > > diff --git a/src/lj_lex.c b/src/lj_lex.c > index 52856912..c66660d7 100644 > --- a/src/lj_lex.c > +++ b/src/lj_lex.c > @@ -138,7 +138,7 @@ static int lex_skipeq(LexState *ls) > int count = 0; > LexChar s = ls->c; > lua_assert(s == '[' || s == ']'); > - while (lex_savenext(ls) == '=') > + while (lex_savenext(ls) == '=' && count < 0x20000000) > count++; > return (ls->c == s) ? count : (-count) - 1; > } > diff --git a/test/tarantool-tests/lj-812-too-long-string-separator.test.lua b/test/tarantool-tests/lj-812-too-long-string-separator.test.lua > new file mode 100644 > index 00000000..fda69d17 > --- /dev/null > +++ b/test/tarantool-tests/lj-812-too-long-string-separator.test.lua > @@ -0,0 +1,31 @@ > +local tap = require('tap') > + > +-- Test to check that we avoid parsing of too long separator Typo: s/parsing of/parsing/ Typo: s/separator/separators/ > +-- for long strings. > +-- See also the discussion in the > +-- https://github.com/LuaJIT/LuaJIT/issues/812. > + > +local test = tap.test('lj-812-too-long-string-separator'):skipcond({ > + ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'), Please write a more detailed description of how it can be tested for non-GC64 build and why it is disabled now, as we have discussed offline. > +}) > +test:plan(2) > + > +-- We can't check the string overflow itself without a really > +-- large file, because the ERR_MEM error will be raised, due to > +-- the string buffer reallocations during parsing. > +-- Keep such huge file in the repo is pointless, so just check > +-- that we don't parse long string after some separator length. > +-- Be aware that this limit is different for Lua 5.1. Please fix the same typos as in the commit message here. > + > +-- Use the hardcoded limit. The same as in the . > +local separator = string.rep('=', 0x20000000 + 1) > +local test_str = ('return [%s[]%s]'):format(separator, separator) > + > +local f, err = loadstring(test_str, 'empty_str_f') > +test:ok(not f, 'correct status when parsing string with too long separator') > + > +-- Check error message. > +test:ok(tostring(err):match('invalid long string delimiter'), > + 'correct error when parsing string with too long separator') > + > +test:done(true) > -- > 2.41.0 > Best regards, Maxim Kokryashkin