* git-send-email.perl defect: address missing trailing > accepted
@ 2009-10-20 22:12 Joe Perches
2009-10-20 22:29 ` Erik Faye-Lund
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2009-10-20 22:12 UTC (permalink / raw)
To: git
I typo cut/pasted an invalid email address,
neglecting to copy the trailing ">".
was: "Name <addr.org"
needed: "Name <addr.org>"
Anyone have suggestions on how to get
git-send-email.perl to notify and abort
sending on more invalid address styles?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: git-send-email.perl defect: address missing trailing > accepted
2009-10-20 22:12 git-send-email.perl defect: address missing trailing > accepted Joe Perches
@ 2009-10-20 22:29 ` Erik Faye-Lund
2009-10-20 22:48 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Erik Faye-Lund @ 2009-10-20 22:29 UTC (permalink / raw)
To: Joe Perches; +Cc: git
On Wed, Oct 21, 2009 at 12:12 AM, Joe Perches <joe@perches.com> wrote:
> I typo cut/pasted an invalid email address,
> neglecting to copy the trailing ">".
>
> was: "Name <addr.org"
> needed: "Name <addr.org>"
>
> Anyone have suggestions on how to get
> git-send-email.perl to notify and abort
> sending on more invalid address styles?
Something along these lines? Of course, the error message is, uhm,
less than helpful :)
--->8---
diff --git a/git-send-email.perl b/git-send-email.perl
index f5ba4e7..83f5e80 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -787,6 +787,10 @@ sub is_rfc2047_quoted {
sub sanitize_address
{
my ($recipient) = @_;
+ if ($recipient =~ m/.*<[^>]*$/) {
+ die "EEK!"
+ }
+
my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
if (not $recipient_name) {
--
Erik "kusma" Faye-Lund
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: git-send-email.perl defect: address missing trailing > accepted
2009-10-20 22:29 ` Erik Faye-Lund
@ 2009-10-20 22:48 ` Joe Perches
2009-10-20 22:56 ` Erik Faye-Lund
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2009-10-20 22:48 UTC (permalink / raw)
To: kusmabite; +Cc: git
On Wed, 2009-10-21 at 00:29 +0200, Erik Faye-Lund wrote:
> On Wed, Oct 21, 2009 at 12:12 AM, Joe Perches <joe@perches.com> wrote:
> > I typo cut/pasted an invalid email address,
> > neglecting to copy the trailing ">".
> > was: "Name <addr.org"
> > needed: "Name <addr.org>"
> > Anyone have suggestions on how to get
> > git-send-email.perl to notify and abort
> > sending on more invalid address styles?
>
> Something along these lines? Of course, the error message is, uhm,
> less than helpful :)
>
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -787,6 +787,10 @@ sub is_rfc2047_quoted {
> sub sanitize_address
> {
> my ($recipient) = @_;
> + if ($recipient =~ m/.*<[^>]*$/) {
> + die "EEK!"
> + }
> +
Maybe this? Seems to work.
diff --git a/git-send-email.perl b/git-send-email.perl
index a0279de..52ddd9e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -374,15 +374,18 @@ my ($repoauthor, $repocommitter);
# Verify the user input
foreach my $entry (@to) {
- die "Comma in --to entry: $entry'\n" unless $entry !~ m/,/;
+ die "Comma in --to entry: '$entry'\n" unless $entry !~ m/,/;
+ die "Invalid --to entry: '$entry'\n" unless $entry !~ m/.*<[^>]*$/;
}
foreach my $entry (@initial_cc) {
- die "Comma in --cc entry: $entry'\n" unless $entry !~ m/,/;
+ die "Comma in --cc entry: '$entry'\n" unless $entry !~ m/,/;
+ die "Invalid --cc entry: '$entry'\n" unless $entry !~ m/.*<[^>]*$/;
}
foreach my $entry (@bcclist) {
- die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/;
+ die "Comma in --bcclist entry: '$entry'\n" unless $entry !~ m/,/;
+ die "Invalid --bcclist entry: '$entry'\n" unless $entry !~ m/.*<[^>]*$/;
}
sub parse_address_line {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: git-send-email.perl defect: address missing trailing > accepted
2009-10-20 22:48 ` Joe Perches
@ 2009-10-20 22:56 ` Erik Faye-Lund
2009-10-20 23:00 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Erik Faye-Lund @ 2009-10-20 22:56 UTC (permalink / raw)
To: Joe Perches; +Cc: git
On Wed, Oct 21, 2009 at 12:48 AM, Joe Perches <joe@perches.com> wrote:
>> Something along these lines? Of course, the error message is, uhm,
>> less than helpful :)
>
> Maybe this? Seems to work.
>
Didn't my version work for you? It worked for me.
I find it a bit cleaner to make it a part of the address-sanitizion,
since that needs to be performed for all addresses. I might miss
something vital, though. I don't really speak perl all that well ;)
--
Erik "kusma" Faye-Lund
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: git-send-email.perl defect: address missing trailing > accepted
2009-10-20 22:56 ` Erik Faye-Lund
@ 2009-10-20 23:00 ` Joe Perches
2009-10-20 23:28 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2009-10-20 23:00 UTC (permalink / raw)
To: kusmabite; +Cc: git
On Wed, 2009-10-21 at 00:56 +0200, Erik Faye-Lund wrote:
> Didn't my version work for you? It worked for me.
Hi Erik.
It worked, but an unexplained die isn't great,
so I put it where the other validations are done.
It seems that the regex for address validation
isn't very good and perhaps there could/should
be a stronger validation done for each address
entered.
cheers, Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: git-send-email.perl defect: address missing trailing > accepted
2009-10-20 23:00 ` Joe Perches
@ 2009-10-20 23:28 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2009-10-20 23:28 UTC (permalink / raw)
To: Joe Perches; +Cc: kusmabite, git
Joe Perches <joe@perches.com> writes:
> It seems that the regex for address validation
> isn't very good and perhaps there could/should
> be a stronger validation done for each address
> entered.
The existing ones are actually already harmful. It would trigger on a
valid addresses like this, wouldn't it?
To: "Hamano, Jun" <gitster@pobox.com>
It is worse than that.
The "# Verify the user input" block is in a wrong place in the codepath.
After @to goes through this bogus "verification" step, it then is given to
expand_aliases(), which may expand to real addresses. And then they pass
through sanitize_address() before getting used.
Three implications that come from this wrong code structure are:
(1) The stricter checks you added on top of the existing bogus
verification step may prevent @to to reach expand_aliases() step,
even if the tokens in @to may expand to correct addresses by this
expand_aliases() step if they were allowed to reach here;
(2) The result from expand_aliases() is never checked.
(3) The result from sanitize_address() is not checked either.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-10-20 23:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-20 22:12 git-send-email.perl defect: address missing trailing > accepted Joe Perches
2009-10-20 22:29 ` Erik Faye-Lund
2009-10-20 22:48 ` Joe Perches
2009-10-20 22:56 ` Erik Faye-Lund
2009-10-20 23:00 ` Joe Perches
2009-10-20 23:28 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).