Prevent private web service methods from being called

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
WebService
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Koosha KM, Assigned: Koosha KM)

Tracking

unspecified
Bugzilla 4.2
Bug Flags:
approval +
approval4.4 +
approval4.2 +

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 664188 [details] [diff] [review]
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.
Attachment #664188 - Flags: review?(LpSolit)

Comment 1

5 years ago
It should not be possible to call a private method. If we can, that's a bug.
Severity: enhancement → normal
Target Milestone: Bugzilla 4.4 → Bugzilla 4.2
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
(Assignee)

Updated

5 years ago
Attachment #664188 - Flags: review?(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.
Attachment #664188 - Flags: review?(dkl) → review+

Updated

5 years ago
Attachment #664188 - Flags: review?(LpSolit)

Comment 4

5 years ago
(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.
Flags: approval4.4+
Flags: approval4.2+
Flags: approval+

Comment 5

5 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/WebService/Server.pm
Committed revision 8422.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/WebService/Server.pm
Committed revision 8416.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/WebService/Server.pm
Committed revision 8149.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.