Bug 63885 - shifting into sign bit in mod_access_compat.c is undefined behaviour
Summary: shifting into sign bit in mod_access_compat.c is undefined behaviour
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_access_compat (show other bugs)
Version: 2.4.41
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-26 20:55 UTC by Paul Dreik
Modified: 2020-09-08 12:38 UTC (History)
1 user (show)


Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Dreik 2019-10-26 20:55:36 UTC 
I built apache with undefined behaviour sanitizer on, and the the resulting binary complains on this row: 
https://github.com/apache/httpd/blob/402ea113bbd93eef00e66ba0caaef75df15cd0e8/modules/aaa/mod_access_compat.c#L112

It gives the following error:
 okt 26 21:40:02 torsken apachectl[27317]: mod_access_compat.c:112:43: runtime error: left shift of 1 by 63 places cannot be represented in type 'long int'

The above was made by modifying the debian package instead of using the upstream version but it seems relevant also on 2.4.41.

I tried to build the latest version from svn, but it failed when I tried to point out the path to the apr executable to buildconf and I gave up after a while.
Comment 1 Christophe JAILLET 2019-10-27 06:40:04 UTC 
Thx for the report.

We should use apr_uint64_t instead of apr_int64_t for 'limited' in the structure 'cmd_parms_struct'.

Fix for trunk is trivial but backport for 2.4.x is unlikely, because of compatibility reasons (at least IMHO).
Comment 2 Yann Ylavic 2019-10-27 11:08:28 UTC 
I think that it's AP_METHOD_BIT which should be apr_uint64_t.
Comment 3 Yann Ylavic 2019-10-27 11:25:22 UTC 
Possibly, something backportable would be to:

#define AP_METHOD_UBIT ((apr_uint64_t)1)

and use that in our codebase in place of AP_METHOD_BIT (now deprecated)...
Comment 4 Joe Orton 2020-09-08 12:38:37 UTC 
I hadn't seen this before, but discovered it and fixed it in r1874114 on trunk.  IMO probably not worth backporting since it's only triggering ubsan.