You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by sf...@apache.org on 2009/10/24 15:29:03 UTC
svn commit: r829362 - in /httpd/httpd/trunk: CHANGES
modules/cache/mod_socache_shmcb.c
Author: sf
Date: Sat Oct 24 13:29:03 2009
New Revision: 829362
URL: http://svn.apache.org/viewvc?rev=829362&view=rev
Log:
Only allow parens in filename if cachesize is given. Return error otherwise
to catch missing parens.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=829362&r1=829361&r2=829362&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sat Oct 24 13:29:03 2009
@@ -10,8 +10,8 @@
mod_proxy_ftp: NULL pointer dereference on error paths.
[Stefan Fritsch <sf fritsch.de>, Joe Orton]
- *) mod_socache_shmcb: Only parse cache size in parens at the end of the
- string. Fixes SSLSessionCache directive mis-parsing parens in pathname.
+ *) mod_socache_shmcb: Allow parens in file name if cache size is given.
+ Fixes SSLSessionCache directive mis-parsing parens in pathname.
PR 47945. [Stefan Fritsch]
*) htpasswd: Improve out of disk space handling. PR 30877. [Stefan Fritsch]
Modified: httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c?rev=829362&r1=829361&r2=829362&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c Sat Oct 24 13:29:03 2009
@@ -280,11 +280,20 @@
cp = strrchr(path, '(');
cp2 = path + strlen(path) - 1;
- if (cp && (*cp2 == ')')) {
+ if (cp) {
+ char *endptr;
+ if (*cp2 != ')') {
+ return "Invalid argument: no closing parenthesis or cache size "
+ "missing after pathname with parenthesis";
+ }
*cp++ = '\0';
*cp2 = '\0';
- ctx->shm_size = atoi(cp);
+
+ ctx->shm_size = strtol(cp, &endptr, 10);
+ if (endptr != cp2) {
+ return "Invalid argument: cache size not numerical";
+ }
if (ctx->shm_size < 8192) {
return "Invalid argument: size has to be >= 8192 bytes";
@@ -299,6 +308,9 @@
}
}
+ else if (cp2 >= path && *cp2 == ')') {
+ return "Invalid argument: no opening parenthesis";
+ }
return NULL;
}
Re: svn commit: r829362 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_socache_shmcb.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/24/2009 08:58 PM, Stefan Fritsch wrote:
> On Sat, 24 Oct 2009, Ruediger Pluem wrote:
>>> Author: sf
>>> Date: Sat Oct 24 13:29:03 2009
>>> New Revision: 829362
>>>
>>> URL: http://svn.apache.org/viewvc?rev=829362&view=rev
>>> Log:
>>> Only allow parens in filename if cachesize is given. Return error
>>> otherwise
>>> to catch missing parens.
>>>
>>> Modified:
>>> httpd/httpd/trunk/CHANGES
>>> httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
>>>
>>
>>> Modified: httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c?rev=829362&r1=829361&r2=829362&view=diff
>>>
>>> ==============================================================================
>>>
>>> --- httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c (original)
>>> +++ httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c Sat Oct 24
>>> 13:29:03 2009
>>> @@ -280,11 +280,20 @@
>>>
>>> cp = strrchr(path, '(');
>>> cp2 = path + strlen(path) - 1;
>>> - if (cp && (*cp2 == ')')) {
>>> + if (cp) {
>>> + char *endptr;
>>> + if (*cp2 != ')') {
>>> + return "Invalid argument: no closing parenthesis or
>>> cache size "
>>> + "missing after pathname with parenthesis";
>>> + }
>>
>> Doesn't this bring us back to the start where filenames with ( ) are
>> not allowed?
>
> Now we consistently allow ( ) in the file name if the cache size is
> given at the end. Before my changes, we never allowed ( ) in the file
> name, even if there was an additional (NUMBERS) at the end.
Ok, now I get it better. This is also fine with me.
Regards
Rüdiger
Re: svn commit: r829362 - in /httpd/httpd/trunk: CHANGES
modules/cache/mod_socache_shmcb.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sat, 24 Oct 2009, Ruediger Pluem wrote:
>> Author: sf
>> Date: Sat Oct 24 13:29:03 2009
>> New Revision: 829362
>>
>> URL: http://svn.apache.org/viewvc?rev=829362&view=rev
>> Log:
>> Only allow parens in filename if cachesize is given. Return error otherwise
>> to catch missing parens.
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
>>
>
>> Modified: httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c?rev=829362&r1=829361&r2=829362&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c (original)
>> +++ httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c Sat Oct 24 13:29:03 2009
>> @@ -280,11 +280,20 @@
>>
>> cp = strrchr(path, '(');
>> cp2 = path + strlen(path) - 1;
>> - if (cp && (*cp2 == ')')) {
>> + if (cp) {
>> + char *endptr;
>> + if (*cp2 != ')') {
>> + return "Invalid argument: no closing parenthesis or cache size "
>> + "missing after pathname with parenthesis";
>> + }
>
> Doesn't this bring us back to the start where filenames with ( ) are not allowed?
Now we consistently allow ( ) in the file name if the cache size is given
at the end. Before my changes, we never allowed ( ) in the file name,
even if there was an additional (NUMBERS) at the end.
> So somehere/some(parens)path is now forbidden. Why?
> I would say that only the following patterns should be invalid:
>
> somehere/some(NONUMBERS
> somehere/some(NONUMBERS)
> somehere/some(NUMBERS
>
> everything else should be allowed and if we do end the string with (NUMBERS)
> it should be assumed that no cache size was given
You mean "unless we do end the string with (NUMBERS)"?
I think the two variants I proposed are more predictable. Either allow
everything (which has the disadvantage that typos are not caught) or don't
allow parens in the path at all unless we also have a cache size.
But I don't really mind that much. If you think your variant is better,
I am fine with that, too.
Stefan
Re: svn commit: r829362 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_socache_shmcb.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/24/2009 03:29 PM, sf@apache.org wrote:
> Author: sf
> Date: Sat Oct 24 13:29:03 2009
> New Revision: 829362
>
> URL: http://svn.apache.org/viewvc?rev=829362&view=rev
> Log:
> Only allow parens in filename if cachesize is given. Return error otherwise
> to catch missing parens.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
>
> Modified: httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c?rev=829362&r1=829361&r2=829362&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c (original)
> +++ httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c Sat Oct 24 13:29:03 2009
> @@ -280,11 +280,20 @@
>
> cp = strrchr(path, '(');
> cp2 = path + strlen(path) - 1;
> - if (cp && (*cp2 == ')')) {
> + if (cp) {
> + char *endptr;
> + if (*cp2 != ')') {
> + return "Invalid argument: no closing parenthesis or cache size "
> + "missing after pathname with parenthesis";
> + }
Doesn't this bring us back to the start where filenames with ( ) are not allowed?
So somehere/some(parens)path is now forbidden. Why?
I would say that only the following patterns should be invalid:
somehere/some(NONUMBERS
somehere/some(NONUMBERS)
somehere/some(NUMBERS
everything else should be allowed and if we do end the string with (NUMBERS)
it should be assumed that no cache size was given
Regards
Rüdiger