threads / patch / 42961

patchgitweb: escape link body in format_ref_marker

Subject: [PATCH] gitweb: escape link body in format_ref_marker

## tl;dr

4 messages between Jul 29, 2016 and Aug 2, 2016. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Andreas Brauchli· Jul 29, 2016, 14:49 UTC · lore

Fix a case where an html link can be generated from unescaped input resulting in invalid strict xhtml or potentially injected code.

An overview of a repo with a tag "1.0.0&0.0.1" would previously result in an unescaped amperstand in the link body.

Signed-off-by: Andreas Brauchli <a.brauchli@elementarea.net>
---
 gitweb/gitweb.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to gitweb/gitweb.perl +1 −1
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2fddf75..33d701d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2090,7 +2090,7 @@ sub format_ref_marker {
                                -href => href(
                                        action=>$dest_action,
                                        hash=>$dest
-                               )}, $name);
+                               )}, esc_html($name));

                        $markers .= " <span
class=\"".esc_attr($class)."\" title=\"".esc_attr($ref)."\">" .
                                $link . "</span>";

-- 
2.7.4
Jakub Narębski· Jul 30, 2016, 17:46 UTC · re: Andreas Brauchli · lore

Re: [PATCH] gitweb: escape link body in format_ref_marker

W dniu 29.07.2016 o 16:49, Andreas Brauchli pisze:
Show 5 quoted lines
> Fix a case where an html link can be generated from unescaped input
> resulting in invalid strict xhtml or potentially injected code.
> 
> An overview of a repo with a tag "1.0.0&0.0.1" would previously result
> in an unescaped amperstand in the link body.
s/amperstand/ampersand/
        ^---------------- [ spurious 't' ]

Unescaped ampersand would cause trouble if gitweb output is in XHTML mode, as some web browsers are strict about validating XML, and do not display anything (note: this detail is unnecessary in the commit message).

I guess that "1.0.0&0.0.1" tags are something encountered in real repositories, while "1<b>2" would be just something evil...

> 
> Signed-off-by: Andreas Brauchli <a.brauchli@elementarea.net>
Good catch!
Acked-by: Jakub Narębski <jnareb@gmail.com>
Show 19 quoted lines
> ---
>  gitweb/gitweb.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 2fddf75..33d701d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2090,7 +2090,7 @@ sub format_ref_marker {
>                                 -href => href(
>                                         action=>$dest_action,
>                                         hash=>$dest
> -                               )}, $name);
> +                               )}, esc_html($name));
> 
>                         $markers .= " <span
> class=\"".esc_attr($class)."\" title=\"".esc_attr($ref)."\">" .
>                                 $link . "</span>";
>
Junio C Hamano· Aug 1, 2016, 19:54 UTC · re: Jakub Narębski · lore

Re: [PATCH] gitweb: escape link body in format_ref_marker

Jakub Narębski <jnareb@gmail.com> writes:
> Good catch!
>
> Acked-by: Jakub Narębski <jnareb@gmail.com>

Sigh; the contents may be good but the patch is unusable as-is because of heavy whitespace damage.

I'll fix it up.  Thanks, both.
Show 19 quoted lines
>> ---
>>  gitweb/gitweb.perl | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 2fddf75..33d701d 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -2090,7 +2090,7 @@ sub format_ref_marker {
>>                                 -href => href(
>>                                         action=>$dest_action,
>>                                         hash=>$dest
>> -                               )}, $name);
>> +                               )}, esc_html($name));
>> 
>>                         $markers .= " <span
>> class=\"".esc_attr($class)."\" title=\"".esc_attr($ref)."\">" .
>>                                 $link . "</span>";
>>
Andreas Brauchli· Aug 2, 2016, 10:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: escape link body in format_ref_marker

On Mon, Aug 1, 2016 at 9:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> Jakub Narębski <jnareb@gmail.com> writes:
>
>> Good catch!
>>
>> Acked-by: Jakub Narębski <jnareb@gmail.com>
>
> Sigh; the contents may be good but the patch is unusable as-is
> because of heavy whitespace damage.
>
> I'll fix it up.  Thanks, both.

My apologies for that, it seems that gmail doesn't do tabs. I resubmitted the PR as #283 on GitHub along with Jakub's Ack and fixed the typo he pointed out in the commit message

https://github.com/git/git/pull/283

Kind regards andreas

Show 20 quoted lines
>
>>> ---
>>>  gitweb/gitweb.perl | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index 2fddf75..33d701d 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -2090,7 +2090,7 @@ sub format_ref_marker {
>>>                                 -href => href(
>>>                                         action=>$dest_action,
>>>                                         hash=>$dest
>>> -                               )}, $name);
>>> +                               )}, esc_html($name));
>>>
>>>                         $markers .= " <span
>>> class=\"".esc_attr($class)."\" title=\"".esc_attr($ref)."\">" .
>>>                                 $link . "</span>";
>>>

← back to recent threads