From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [RFC 02/19] xen: guestcopy: Provide an helper to copy string from guest Date: Tue, 17 Jun 2014 10:23:15 +0100 Message-ID: <53A00903.2060904@linaro.org> References: <1402935486-29136-1-git-send-email-julien.grall@linaro.org> <1402935486-29136-3-git-send-email-julien.grall@linaro.org> <53A011E4020000780001ADF4@mail.emea.novell.com> <53A005AC.3060306@linaro.org> <53A023E7020000780001AE8B@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Wwpbr-000354-HQ for xen-devel@lists.xenproject.org; Tue, 17 Jun 2014 09:23:19 +0000 Received: by mail-wi0-f169.google.com with SMTP id hi2so6547639wib.4 for ; Tue, 17 Jun 2014 02:23:17 -0700 (PDT) In-Reply-To: <53A023E7020000780001AE8B@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , ian.campbell@citrix.com, tim@xen.org, Ian Jackson , stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org, Daniel De Graaf List-Id: xen-devel@lists.xenproject.org On 17/06/14 10:17, Jan Beulich wrote: >>>> On 17.06.14 at 11:09, wrote: >> On 17/06/14 09:01, Jan Beulich wrote: >>>>>> On 16.06.14 at 18:17, wrote: >>>> + >>>> + /* Add an extra +1 to append \0. We can't assume the guest will >>>> + * provide a valid string */ >>> >>> Now this is the case for flask, but for a generic string copying >>> routine I don't think this is desirable. It seems especially wrong to >>> aid the guest with putting a NUL where none was. If you really >>> want this, I guess you would be better off adding two variants: >>> One which demands the string to be NUL-terminated (in which >>> case passing in a size is sort of bogus), and one which takes a >>> size and inserts a NUL. >> >> A malicious guest could pass a big buffer without a NUL-terminated. If >> we don't limit the size and check the NUL-terminated character the guest >> could respectively exhaust Xen memory and exploit it. >> >> Therefore we can't rely on the guest to provide a valid string. This >> solution will avoid to check in every caller that the string is >> correctly terminated. > > You seem to imply that by not passing in a size I also meant not > passing in a maximum size - I didn't say that, though. You absolutely > have to limit the string length for security reasons, but it's clearly a > difference whether you silently NUL-terminate the value after the > maximum number of characters, or return with an error. I didn't understand in this way your previous mail. Thank you for the explanation. It looks like for my use case it's better to throw an error if we don't have enough place. It would help us if one day the path start to be very long. I'm wondering if we can also make this change for flask... Daniel, any though? Regards, -- Julien Grall