You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Klaus Rennecke <kr...@tigris.org> on 2004/07/07 20:36:07 UTC

[PATCH] Issue #1947 Submission #3 svn_path_uri_decode may copy garbage and overrun buffer when given partial % escape

Branko Čibej wrote:
> Klaus Rennecke wrote:
>> Index: D:/kre/workspace/svn/subversion/libsvn_subr/path.c
>> ===================================================================
>> [...]
>> +#include <ctype.h>                      /* for svn_path_uri_decode() */
>> [...]
> This should be "#include <apr_lib.h>" and "apr_isxdigit". Otherwise, +1.
> We should make a similar change in svn_path_is_uri_safe.

That is fixed with this new patch. I had a look around and noticed that 
isxdigit is used in some places. For instance in fs_fs.c. But I cannot 
judge if that's any problem.

> Hm. I hope noone ever tries to port Subversion to an EBCDIC platform, 
> because not one of these checks will work correctly there... :-\

I'm now directly comparing to ASCII values, so this should not be an 
issue anymore. Keeps things simple as well. :-)

Should I try to squeeze the new patches onto the issue tracker?

[[[
Check that the two characters following the % escape are valid hex 
digits. This serves to check for premature end of input as well.

Fixes Issue #1947 svn_path_uri_decode may copy garbage and overrun 
buffer when given partial % escape.

* subversion/libsvn_subr/path.c
   (svn_path_uri_decode): Check syntax of % escape.

* subversion/tests/libsvn_subr/path-test.c
   (test_uri_decode): New test function.
   (test_funcs): Added test_uri_decode.
]]]

/Klaus

Feature request: relative patch creation for subclipse (Was Re: [PATCH] Issue #1947 [...])

Posted by Klaus Rennecke <kr...@tigris.org>.
Ben Reser wrote:
> [...]   Please make your patches relative to the trunk dir.
> It just makes it so much easier to apply becuase I don't have to count
> how many paths I need to tell patch to rip off...
> 

I can see that. So eclipse patches are OUT. It already irked me that 
they contain absolute path names.

It would be nice to have a preference setting in subclipse that would 
make the file names in patches relative to the project root. Or, more 
flexible, to the patch start directory.

Hence, redirected to subclipse.

However, this does not seem to be as easy as it sounds. I don't see how 
to set the base directory for javahl execution. Maybe I can bend the 
command line integration to do that, as an experiment. :-)

/Klaus

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue #1947 Submission #4 svn_path_uri_decode may copy garbage and overrun buffer when given partial % escape

Posted by Ben Reser <be...@reser.org>.
On Thu, Jul 08, 2004 at 11:45:50AM +0200, Klaus Rennecke wrote:
> Branko ??ibej wrote:
> 
> >[...] macros that evaluate the argument more than once are evil.
> 
> 
> Ow. That naïve macro of mine was asking for more trouble than there was 
> before.
> 
> >[...]
> >Just fix the patch and I'll apply it. :-)
> 
> 
> Yessir! :-)
> 
> /Klaus
> 
> [[[
> Check that the two characters following the % escape are valid hex digits. 
> This serves to check for premature end of input as well.
> 
> Fixes Issue #1947 svn_path_uri_decode may copy garbage and overrun buffer 
> when given partial % escape.
> 
> * subversion/libsvn_subr/path.c
>  (svn_path_uri_decode): Check syntax of % escape.
> 
> * subversion/tests/libsvn_subr/path-test.c
>  (test_uri_decode): New test function.
>  (test_funcs): Added test_uri_decode.
> ]]]
> 

Committed in r10199 and r10211 (two commits because I missed this email
and the test suite).

One other comment.  Please make your patches relative to the trunk dir.
It just makes it so much easier to apply becuase I don't have to count
how many paths I need to tell patch to rip off...

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue #1947 Submission #4 svn_path_uri_decode may copy garbage and overrun buffer when given partial % escape

Posted by Klaus Rennecke <kr...@tigris.org>.
Branko Čibej wrote:

> [...] macros that evaluate the argument more than once are evil.


Ow. That naïve macro of mine was asking for more trouble than there was 
before.

> [...]
> Just fix the patch and I'll apply it. :-)


Yessir! :-)

/Klaus

[[[
Check that the two characters following the % escape are valid hex digits. This serves to check for premature end of input as well.

Fixes Issue #1947 svn_path_uri_decode may copy garbage and overrun buffer when given partial % escape.

* subversion/libsvn_subr/path.c
  (svn_path_uri_decode): Check syntax of % escape.

* subversion/tests/libsvn_subr/path-test.c
  (test_uri_decode): New test function.
  (test_funcs): Added test_uri_decode.
]]]





Re: [PATCH] Issue #1947 Submission #3 svn_path_uri_decode may copy garbage and overrun buffer when given partial % escape

Posted by Branko Čibej <br...@xbc.nu>.
Klaus Rennecke wrote:

> I'm now directly comparing to ASCII values, so this should not be an 
> issue anymore. Keeps things simple as well. :-)

Don't do that. Just use apr_isxdigit, otherwise we'd give a false sense 
of supporting platforms with native encodings that aren't ASCII- based. 
Besides, macros that evaluate the argument more than once are evil.

> Should I try to squeeze the new patches onto the issue tracker?

Just fix the patch and I'll apply it. :-)

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org