On Thu, Aug 6, 2026 at 6:50 AM Ruediger Pluem <[email protected]> wrote:
>
>
>
> On 8/3/26 2:30 PM, [email protected] wrote:
> > Author: covener
> > Date: Mon Aug 3 12:30:34 2026
> > New Revision: 1936811
> >
> > Log:
> > apr_memcache: error checking
> >
> > Reviewed By: covener, jorton, jfclere
> >
> > Modified:
> > apr/apr/trunk/memcache/apr_memcache.c
> >
> > Modified: apr/apr/trunk/memcache/apr_memcache.c
> > ==============================================================================
> > --- apr/apr/trunk/memcache/apr_memcache.c Mon Aug 3 12:28:37 2026
> > (r1936810)
> > +++ apr/apr/trunk/memcache/apr_memcache.c Mon Aug 3 12:30:34 2026
> > (r1936811)
> >
> > @@ -1169,9 +1174,14 @@ apr_memcache_version(apr_memcache_server
> > }
> >
> > if (strncmp(MS_VERSION, conn->buffer, MS_VERSION_LEN) == 0) {
> > - *baton = apr_pstrmemdup(p, conn->buffer+MS_VERSION_LEN+1,
> > - conn->blen - MS_VERSION_LEN - 2);
> > - rv = APR_SUCCESS;
> > + if (conn->blen < MS_VERSION_LEN + 2) {
> > + rv = APR_EGENERAL;
> > + }
> > + else {
> > + *baton = apr_pstrmemdup(p, conn->buffer+MS_VERSION_LEN+1,
> > + conn->blen - MS_VERSION_LEN - 2);
>
> I know that this is taken from the old code, but shouldn't it be -3 or -1
> instead of -2 above?
>
> MS_VERSION is "VERSION"
> Hence MS_VERSION_LEN is 7
>
> If memcached correctly replies to the "version" command it returns:
>
> VERSION XYZ\r\n
>
> This means conn->blen is 13
>
> conn->buffer+MS_VERSION_LEN+1 = conn->buffer+7+1 = conn->buffer+8 points to
> the 'X'
> conn->blen - MS_VERSION_LEN - 2 is 13 - 7 - 2 = 4
>
> This would mean baton would be "XYZ\r". This sounds wrong. It should be
> either "XYZ" (I would prefer this,
> in this case we need -3 above) or "XYZ\r\n" (in this case we need -1 above).
I think you are right (-3). Is the length check also off? by 2?
if (strncmp(MS_VERSION, conn->buffer, MS_VERSION_LEN) == 0) {
if (conn->blen < MS_VERSION_LEN + 2) {
The buffer needs space for VERSION + SP + CR + LF _and_ at least 1
character/digit.
