*) core: make ap_escape_quotes() work correctly on strings #298
Conversation
icing
commented
Mar 17, 2022
icing
commented
with more than MAX_INT/2 characters, counting quotes double.
Credit to <generalbugs@zippenhop.com> for finding this.
|
Added in trunk as r1899609. |
| @@ -2615,7 +2615,7 @@ AP_DECLARE(void) ap_content_type_tolower(char *str) | |||
| */ | |||
| AP_DECLARE(char *) ap_escape_quotes(apr_pool_t *p, const char *instring) | |||
| { | |||
| int newlen = 0; | |||
| apr_ssize_t extra = 0; | |||
| const char *inchr = instring; | |||
There was a problem hiding this comment.
Nitpick: Why apr_ssize_t and not apr_size_t?
There was a problem hiding this comment.
extra is used in a ptrdiff_t expression below, which is signed. apr_ssize_t is ptrdiff_t on all but the most ancients of platforms.
But is it correct to use? The question is, does it deal correctly with strings that are longer than ptrdiff_t can hold (lets call this PTRDIFF_MAX)?
- an
instringthat is longer than PTRDIFF_MAX: this would mean trouble as(inchr - instring)in the palloc expression would overflow. Which is undefined behaviour in C. C defines that you can substract pointers into
the same array to get a number. So, maybe we can ignore that one. - an
instringthat is almost as long as PTRDIFF_MAX with enough quote characters that the resulting length is larger than PTRDIFF_MAX. This is currently not checked.
I think we need a check that the expression (inchr - instring) + extra + 1 did not overflow, e.g. became < 0, to be totally safe. And for this, we need to calculdate in apr_ssize_t, I guess.
What do you think?
| if (!extra) { | ||
| return apr_pstrdup(p, instring); | ||
| } | ||
|
|
There was a problem hiding this comment.
Maybe apr_pmemdup(), as we can easily compute the length, as done below.
