Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115505
To: PHP internals <internals@lists.php.net>
Thread-Topic: [PHP-DEV] [RFC] [VOTE] is_literal
Thread-Index: AQHXccmyKTmd0jicgUa5JF7Uc1BfmqtKvPKAgABLO0E=
Date: Tue, 20 Jul 2021 00:23:47 +0000
Message-ID:
 <DM6PR07MB6618C632D202A8CE41BFCE69F9E19@DM6PR07MB6618.namprd07.prod.outlook.com>
References:
 <CAFv4g+HRHrvCOTgpbVfCXj06De1kL+1+RwnGgq6tcyLNAtLfQA@mail.gmail.com>,<CAFv4g+Fs-iSHoykGhkzzJFZSd=UDB9ZGEq-aeo0VyPjqrppQKw@mail.gmail.com>
In-Reply-To:
 <CAFv4g+Fs-iSHoykGhkzzJFZSd=UDB9ZGEq-aeo0VyPjqrppQKw@mail.gmail.com>
Accept-Language: en-CA, en-US
Content-Language: en-CA
x-ms-exchange-messagesentrepresentingtype: 1
x-tmn: [13+f905dKa9MMAg9hpdJm2ql6tZR3kPvyBCZtnRvaZ9ZaZ7Rz1BWqvu6zteY19mX]
x-ms-publictraffictype: Email
x-incomingheadercount: 44
x-eopattributedmessage: 0
x-ms-office365-filtering-correlation-id: 189ad022-5f4b-4895-54a7-08d94b14a46f
x-ms-traffictypediagnostic: MW2NAM10HT118:
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info:
 qamqza0qxUAujk1KRmptyWiURYrOHCN+zGyi1QkC4RBxXqBfC9x0J+OEfqyyf8QEGMQvxxQD37ZraQtBMu4nBYgFQ00x1sLTwyxdsK4X2GheiWr9c08eRan8qff/2etZU2IK3f4zyWj9nz2HniVxHXmq0Ttr5pfj6qVH9ARasNDE2+Bza7m3tGpvzr/KTYgv9i1afSRhXT2DzfE2+GugjeY2T3QcM2w0/6nEg5uqWDZHsQ5E7+F6KvgpbJB9v12wKZn/E+qcpe5Ha2Ntq+VNKZEu9Oviupks2w+ggNoNFA1Jxg2B5UdBVl0zMuSCRglzlVc1Ei8s9NAnu/6P1Cwkeivxwc/dK+KWyQ8wCvc9ayYjE1pic+95xQij6TX8967TvPxRC4oNpzbtGmAOD/k+w3RaZdrgNLdOGvEZqOfYZUBHXEIOmc5XBxMaH/zoIWMZDjt6xzQ1lgzIG+BuEcLIDHbBqAOAZBlIwJoDXUA3rYc=
x-ms-exchange-antispam-messagedata:
 SmccnN8rWZQbmr5b9SZbXNjrA+ZBfNH5tJ5/AjDf3aw2p6x2gU1JvrrnaH6lIPZ8tjSczAPr9BSf3mva7fI5mOdfqT4jjoq5TfsKGDZGikLYcRJkylaZ/+DzPwiTjnVx6r59P3h70kvNpK2ADEhiT6aVQ1jr1uLhqrh3M/85A9BqcEVjuCCsyHp8RPa17VFa1ZO/A1wGH/V6VT1ngPDoow==
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: hotmail.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-AuthSource: MW2NAM10FT004.eop-nam10.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 189ad022-5f4b-4895-54a7-08d94b14a46f
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jul 2021 00:23:47.9364
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Internet
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW2NAM10HT118
Subject: Re: [PHP-DEV] [RFC] [VOTE] is_literal
From: tysonandre775@hotmail.com (tyson andre)

Hi Craig Francis,=0A=
=0A=
> As an aside, only 4 of 23 'no' voters provided any comment as to why they=
=0A=
> voted that way on the mailing list, which I feel undermines the point of=
=0A=
> the Request For Comment process, with an additional 5 responding personal=
ly=0A=
> off-list after prompting. This makes it harder (or impossible) for points=
=0A=
> to be discussed and addressed.=0A=
=0A=
1. My earlier comments about static analysis, and on behavior depending on =
whether opcache is enabled=0A=
2. This might prevent certain optimizations in the future. For example, cur=
rently, 1-byte strings are all interned to save memory.=0A=
    If is_literal was part of php prior to proposing that optimization, the=
n that optimization may be rejected.=0A=
3. PHP's `string` type is used both for (hopefully) valid unicode strings a=
nd for low level operations on literal byte arrays (e.g. cryptogrophy).=0A=
    It seems really, really strange for a type system to track trustedness =
for a low level primitive to track byte arrays. (the php 6 unicode string r=
efactoring failed)=0A=
=0A=
    Further, if this were to be extended in the future beyond the original =
proposal (e.g. literal strings that are entirely digits are automatically i=
nterned or marked as trusted),=0A=
    this might open previously safe code acting on byte arrays to side chan=
nel issues such as timing attacks (https://en.wikipedia.org/wiki/Timing_att=
ack)=0A=
4. Internal functions and userland polyfills for those functions may uninte=
ntionally differ significantly for the resulting taintedness,=0A=
    e.g. base64_decode in userland being built up byte by byte would end up=
 being possibly untainted?=0A=
5. The fact that 1-byte strings are almost always interned seems like a not=
iceable inconsistency (though library authors can deal with it once they're=
 aware of it), though for it to become an issue a library may need to take =
multiple strings as input=0A=
    (e.g. a contrived example`"echo -- " . $trustedPrefix . shell_escape($n=
otTrusted)` for $trustedPrefix of "'" (or "\n") and $notTrusted of "; evalu=
ate command"=0A=
6. Including it in core would make it harder to remove later if it interfer=
ed with opcache or jit work, or to migrate code to alternative interpreters=
 for php if those were ever implemented (if frameworks were to extensively =
depend on is_literal)=0A=
7. Tracking whether a string is untrusted is a definition only suitable for=
 a few (extremely common) formats for php. But for less common features, ev=
en stringified integers may be a problem (e.g. binary file formats, etc)=0A=
    =0A=
    This is relatively minor given that php is typically used in a web prog=
ramming context with json or html or js/css output=0A=
=0A=
    I'd think is_interned()/intern_string() is much closer to tracking some=
thing that corresponds with php's internals (e.g. and may allow saving memo=
ry in long-running processes which receive duplicate strings as input), tho=
ugh the 10 people who wanted fully featured trustedness checking would prob=
ably want is_literal instead=0A=
8. Serializing and unserializing data would lose information about trustedn=
ess of inputs, unpredictably (e.g. unserialize() in php 8.0 interns array k=
eys).=0A=
=0A=
    There's no (efficient) way to change trusted strings to untrusted or vi=
ce versa, though there are inefficient workarounds (modifying a byte and re=
storing it to stop trusting it, imploding single characters to create a tru=
sted string)=0A=
=0A=
    This may done implicitly in frameworks using APCu/memcached/redis as a =
cache=0A=
=0A=
    (I definitely don't think the serialization data format should track is=
_literal())=0A=
=0A=
9. Future refactorings, optimizations or deoptimizations (or security fixes=
) to unserialize(), etc. may unexpectedly break code using is_literal that =
throw instead of warn (more bug reports, discourage users from upgrading, e=
tc.)=0A=
10. This RFC adds an unknown amount of future work for php-src and PECLs to=
 *intuitively* support mapping trusted inputs to trusted outputs or vice ve=
rsa - less commonly used or unmaintained functions may not behave as expect=
ed for a while=0A=
11. https://pecl.php.net/package/taint is available already for a use case =
with some overlap for setups that need this=0A=
=0A=
=0A=
Aside: I'd have to wonder if ZSTR_IS_INTERNED (and the function to make an =
interned string) would make sense to expose in a PECL as a regular `extensi=
on` (not a `zend_extension`) if is_interned also fails.=0A=
Unlike the zend_extension for https://www.php.net/manual/en/function.is-tai=
nted.php ,=0A=
something simple may be possible without needing the performance hit and=0A=
future conflicts with XDebug that I assume https://www.php.net/manual/en/fu=
nction.is-tainted.php would be prone to.=0A=
(https://pecl.php.net/package/taint seems to use a separate bit to track th=
is. The latest release of the Taint pecl fixes XDebug compatibility)=0A=
=0A=
- Other languages, such as Java, have exposed this for memory management pu=
rposes (rather than security) though it's rarely used directly or in framew=
orks, e.g. https://docs.oracle.com/javase/10/docs/api/java/lang/String.html=
#intern()=0A=
  (https://docs.python.org/3/library/sys.html#sys.intern)=0A=
=0A=
Thanks,=0A=
Tyson=