From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.65.1571665615789970982 for ; Mon, 21 Oct 2019 06:46:56 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=EyL/6E1K; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1571665614; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=j82AyvRedj1Fmmhe1eCjhQtwdtBE6krwG1ZAIHHQXyI=; b=EyL/6E1KOWhEmtInil7Xiai3pbTc0jRvA80SoiulqF2p6ECvUpkUFS9+EqdnD/wlR0iXGc ZqPVAOxA5F6DPsX0andakQAvVFTthc9NgywUdy6ReH160vTAXyMUiqABJCPG3qx/8NCS2e /V3iO5pHHK2AGK4hiWS8Q5GSnlABrzQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-145-R_g6_cqbOtGrVQ3CIqOoIg-1; Mon, 21 Oct 2019 09:46:47 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BBBEC476; Mon, 21 Oct 2019 13:46:45 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-225.ams2.redhat.com [10.36.116.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id D4765608A5; Mon, 21 Oct 2019 13:46:43 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Add PCD to disable safe string constraint assertions To: devel@edk2.groups.io, vit9696@protonmail.com, "Yao, Jiewen" Cc: "Gao, Liming" , "Wang, Jian J" , "Kinney, Michael D" , "Marvin.Haeuser@outlook.com" References: <20191020130553.42851-1-vit9696@protonmail.com> <20191020130553.42851-2-vit9696@protonmail.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E51F6EB@SHSMSX104.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503F80B477@shsmsx102.ccr.corp.intel.com> <6FF18D79-65ED-4C0B-9B03-99C0B5DD539A@protonmail.com> <74D8A39837DF1E4DA445A8C0B3885C503F80B961@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503F80BB78@shsmsx102.ccr.corp.intel.com> <3C39qB2Qd7VT0W_yCclBZSdURrtnV0uckOwSKR_kRX32HbphOSJULDqkQK72u5Fkyi6t_EwsbfWsudml0h4T2ewiAfNm0zATPum9lomAMNk=@protonmail.com> From: "Laszlo Ersek" Message-ID: <3d30addd-de67-8f09-a5b5-9210833f9dd2@redhat.com> Date: Mon, 21 Oct 2019 15:46:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <3C39qB2Qd7VT0W_yCclBZSdURrtnV0uckOwSKR_kRX32HbphOSJULDqkQK72u5Fkyi6t_EwsbfWsudml0h4T2ewiAfNm0zATPum9lomAMNk=@protonmail.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-MC-Unique: R_g6_cqbOtGrVQ3CIqOoIg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 10/21/19 11:58, Vitaly Cheptsov via Groups.Io wrote: > Jiewen, > > We are aware of all these nuances, and you are right that both of them > (strlcpy/strcpy_s) have their downsides. Our opinion is that strlcpy > is more practical, but that is a personal choice, and we do nor care > much on what is available as long as it is documented. > > Our primary issues are not really with string copying, but rather with > other interfaces. For example: > - We use AsciiStrToGuid to convert a string to a GUID in the user > configuration file, and any intentional or unintentionsl typo there > may result in halting the entire app due to length assertion in > DEBUG builds. > - We use AsciiStrCats to append messages to the log buffer as long as > they fit. We do not care what happens when they stop to fit, for us > that means we will have a cut log and a handled error that some > messages did not fit. However, in DEBUG builds that once again > results in halts. > > I am pretty sure there were more, but the use cases are be pretty > similar, so you most likely get an idea. We do handle the error where > necessary, yet we do not expect the code to halt before we can handle > the error. (Since I've been CC'd somewhere mid-thread, I'll offer an opinion...) - For copying strings, I'm with Ulrich Drepper: http://www.sourceware.org/ml/libc-alpha/2000-08/msg00061.html "Every program which is handling strings has to know how long they are." Because I share that opinion, I've never considered the "safe string" functions extremely helpful. - For formatting strings, and especially for parsing strings, I tend to consider functions that cannot deal, by design, with untrusted input, more or less useless. When I was initially working on OvmfPkg/Library/QemuBootOrderLib, and had to parse some hex strings (as parts of OpenFirmware device paths exported by QEMU), I believe I looked for suitable utility functions in edk2 elsewhere. Ultimately I wrote my own ParseUnitAddressHexList() function. A note specific to my use case (=3D guest firmware): in theory, the host has complete control over the guest, so one might claim that the guest should never sanity check input from the host (=3D the guest should always blindly trust input from the host). That has never sat well with me, although I couldn't really tell why; it's just always felt wrong. Later, my concern has been vindicated in two ways: first, in the ACPI linker/loader client, those sanity checks helped identify QEMU issues (not malicious intent for sure, but mistakes nonetheless); and then, we now have SEV (Secure Encrypted Virtualization), where host software does not have full control over the guest. IMO all parser functions (exposed as general utility functions) should expect malicious input by default. (Another example I've been involved with: the base64 decoder.) Thanks Laszlo