The following issue has been SUBMITTED. ====================================================================== https://www.austingroupbugs.net/view.php?id=1591 ====================================================================== Reported By: eggert Assigned To: ====================================================================== Project: Issue 8 drafts Issue ID: 1591 Category: System Interfaces Type: Error Severity: Objection Priority: normal Status: New Name: Paul Eggert Organization: UCLA User Reference: Section: no section number yet (no draft yet) Page Number: no page number yet (no draft yet) Line Number: no line number yet (no draft yet) Final Accepted Text: ====================================================================== Date Submitted: 2022-06-29 18:39 UTC Last Modified: 2022-06-29 18:39 UTC ====================================================================== Summary: Proposed strlcpy spec has problems Description: In <https://sourceware.org/pipermail/libc-alpha/2022-June/140093.html> Florian Weimer brought to my attention Austin Group Issue 986 <https://www.austingroupbugs.net/view.php?id=986>. Although this issue is labeled as "Editorial" and "Clarification Requested", its resolution took a proposed approach that went well beyond editorial or clarification. The proposed approach overlooks some important technical issues that need to be considered before attempting to impose a standard in the controversial area of standardizing strlcpy and related functions.
Here are some problems with the proposed approach: * Although strlcpy may have been a reasonable hack when introduced in the 1990s, technology has passed it by and we now have less-intrusive and more-reliable approaches such as FORTIFY_SOURCE and AddressSanitizer. * With commonly-available current technologies, a program that uses strlcpy is typically *less* secure than one that uses strcpy, because when a fixed-size buffer would be overrun strlcpy silently corrupts the output data via truncation, whereas strcpy catches and reports the program error. * Even though the call strlcpy(DST, SRC, 2 * sizeof DST) is clearly wrong and dangerous, the proposed spec says the call happens to have well-defined behavior if strlen(SRC) < sizeof DST, and requires a conforming implementation to implement it as if via strlen(strcpy(DST, SRC)). Programmers do not need this requirement, and the unnecessary requirement stands in the way of checking strlcpy calls for common programming errors involving miscalculated sizes. * strlcpy introduces the possibility of denial-of-service attacks. The time cost of strlcpy(DST, SRC, sizeof DST) is O(strlen(SRC)), which makes it vulnerable to a denial-of-service attack when the attacker controls SRC and can specify a long SRC string. In this respect, strlcpy is worse than then already-standard (strncpy(DST, SRC, sizeof DST - 1), DST[sizeof DST - 1] = '\0') which despite its other flaws at least has the virtue of costing only O(sizeof DST), a cost that is under the defender's control and is typically small. * There is no demonstrated need for the wcslcpy and wcslcat functions. For example, there is not a single call to either function in the FreeBSD source code. Requiring these functions of every implementation would be makework and a net minus for the community. * There is no good evidence that modifying code to use strlcpy improves security more than using already-standardized approaches, even if technologies like FORTIFY_SOURCE and AddressSanitizer are not available. Desired Action: The simplest way to address these issues is to revert the proposal and not add strlcpy etc. to the standard. Failing that, at least the following actions should be taken: * To address the denial-of-service problem, drop the requirement that strlcpy must compute the string length of an overlong source. If SRC is too long, allow strlcpy(DST, SRC, sizeof DST) to return any value greater than or equal to sizeof DST. This will support the vast majority of uses of strlcpy, which do not need or use the string length. * Allow strlcpy(DST, SRC, sizeof DST) to trap if SRC is too long. This will allow strlcpy implementations to catch and report programming errors that would otherwise result in silent data corruption via truncation. I just now surveyed FreeBSD's bin directory sources, and found that most of its uses of strlcpy fall into this category. (I am not proposing that FreeBSD change its strlcpy implementation; I'm merely noting that in practice, most strlcpy uses would benefit from allowing strlcpy implementations to trap.) * Allow the implementation of strlcpy(DST, SRC, sizeof DST) to modify all the bytes of DST, rather than only those bytes up through strlen(SRC)+1. This would help debugging implementations catch programming errors like strlcpy(DST, SRC, 2 * sizeof DST). For example, assuming DST has a fixed nonzero size, it should be OK to implement strlcpy(DST, SRC, sizeof DST) via (DST[sizeof DST - 1] = '\0', strlen(strncpy(DST, SRC, sizeof DST - 1)). * Remove the wcslcpy and wcslcat functions from the proposal. The cost of standardizing and supporting these unused functions outweighs any benefit. * Whatever other action is taken, modify the rationale to state clearly the engineering problems that strlcpy is designed to address, and the pros and cons of using strlcpy versus using other popular approaches. This is a problematic area, and readers of the standard cannot be expected to be familiar with the ins and outs. None of these suggested actions would invalidate existing strlcpy implementations, which would not need to be changed. ====================================================================== Issue History Date Modified Username Field Change ====================================================================== 2022-06-29 18:39 eggert New Issue 2022-06-29 18:39 eggert Name => Paul Eggert 2022-06-29 18:39 eggert Organization => UCLA 2022-06-29 18:39 eggert Section => no section number yet (no draft yet) 2022-06-29 18:39 eggert Page Number => no page number yet (no draft yet) 2022-06-29 18:39 eggert Line Number => no line number yet (no draft yet) ======================================================================
