793826 - Prevent private web service methods from being called

Status: RESOLVED FIXED

(Bugzilla :: WebService, defect)

Description

[Koosha KM]

Attachment: patch - v1

Usually, we use methods (subroutines) that start with '_' for internal purposes only and they are somehow private. But, we don't check if the user is trying to call one of these private methods. It is not reasonable to be able to call these methods as a client. In addition, they might cause some security implications. It would be better to prevent these methods from being called.

Comment 1

[Frédéric Buclin]

It should not be possible to call a private method. If we can, that's a bug.

Comment 2

[David Lawrence [:dkl]]

It is possible to do so as I just verified it. 

One way to enforce would be add code to each private method to check if caller is the current $self otherwise die. 

sub _private_method_foo {
    my ($self) = @_;
    ThrowCodeError("private_method_called")
        unless (caller)[0]->isa( ref($self) );
    return "foo";
}

Or make each private method an anonymous code ref and call it that way such as:

my $private_method_foo = sub {
    return "foo";
};

sub public_method_foo {
    my ($self) = @_;
    return $self->$private_method_foo();
}

Of course Koosha's fix works as long as we use the same private method naming convention throughout which so far it looks like we do. So that would be the quickest fix with the least amount of code change for now.

dkl

Comment 3

[David Lawrence [:dkl]]

Comment on attachment 664188 [details] [diff] [review]
patch - v1

Review of attachment 664188 [details] [diff] [review]:
-----------------------------------------------------------------

Works as described and r=dkl unless LpSolit wants to make major changes across all webservice code and do it a different/better way.

Comment 4

[Frédéric Buclin]

(In reply to David Lawrence [:dkl] from comment #3)
> Works as described and r=dkl unless LpSolit wants to make major changes
> across all webservice code and do it a different/better way.

No, I'm fine with the way it's done in this patch.

Comment 5

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/WebService/Server.pm
Committed revision 8422.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/WebService/Server.pm
Committed revision 8416.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified Bugzilla/WebService/Server.pm
Committed revision 8149.