Bug 2608 - Signed overflow in openbsd-compat/strlcpy.c
Summary: Signed overflow in openbsd-compat/strlcpy.c
Status: CLOSED WONTFIX
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: Miscellaneous (show other bugs)
Version: 7.3p1
Hardware: All Linux
: P5 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-24 13:58 AEST by Yuanjie Huang
Modified: 2021-04-23 15:02 AEST (History)
3 users (show)

See Also:


Attachments
test driver of strlcpy to demo the bug. (776 bytes, text/x-csrc)
2016-08-24 13:58 AEST, Yuanjie Huang
no flags Details
proposed fix. (2.79 KB, patch)
2016-08-24 13:59 AEST, Yuanjie Huang
no flags Details | Diff
proposed fix v2 (3.13 KB, patch)
2019-06-17 15:58 AEST, Hongxu Jia
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuanjie Huang 2016-08-24 13:58:41 AEST
Created attachment 2866 [details]
test driver of strlcpy to demo the bug.

Pointer arithmatic results in implementation defined signed integer type, so that 's - src' in strlcpy and others may trigger signed overflow.

When the code is compiled by gcc or clang with -ftrapv option, the overflow would lead to program abort.                                              

$ gcc test.c strlcpy.c -o test -m32 -ftrapv
$ ./test 
Allocating src starting from 0x7fffff40
Trying to copy 0x400 from 0x7fffff40 to 0x9630008
Aborted (core dumped)

The proposed fix is also attached.
Comment 1 Yuanjie Huang 2016-08-24 13:59:34 AEST
Created attachment 2867 [details]
proposed fix.
Comment 2 Darren Tucker 2016-10-15 04:29:51 AEDT
Thanks.  Looking (these files are sourced from OpenBSD, so any changes will need to go upstream first).
Comment 3 Darren Tucker 2016-10-17 04:48:14 AEDT
I applied then, after some discussion with some folks, reverted your change.

The rationale is that C11 6.5.6.9 says:
"""
When two pointers are subtracted, both shall point to elements of the
same array object, or one past the last element of the array object; the
result is the difference of the subscripts of the two array elements.
"""

In these cases the objects are arrays of char so the result is defined,
and we believe that the compiler incorrectly trapping on defined behaviour.

I also found https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63303 ("Pointer subtraction is broken when using -fsanitize=undefined") which seems to support this position.
Comment 4 Hongxu Jia 2019-06-17 15:58:00 AEST
Created attachment 3293 [details]
proposed fix v2
Comment 5 Damien Miller 2020-01-25 22:17:32 AEDT
I agree with Darren's analysis - the existing implementation is compliant with ISO C.
Comment 6 Damien Miller 2021-04-23 15:02:27 AEST
closing resolved bugs as of 8.6p1 release