This project does not seem to handle request data directly as such no vulnerable execution paths were found.
include
, or for example
via PHP's auto-loading mechanism.
These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more
1 | <?php |
||
2 | |||
3 | use Monolog\Logger; |
||
4 | use Nelmio\ApiDocBundle\Annotation\ApiDoc; |
||
5 | use Symfony\Component\HttpFoundation\JsonResponse; |
||
6 | use Symfony\Component\HttpFoundation\Request; |
||
7 | |||
8 | class LeagueOverseerHookController extends PlainTextController |
||
9 | { |
||
10 | /** |
||
11 | * The API version of the server performing the request |
||
12 | * @var int |
||
13 | */ |
||
14 | private $version; |
||
15 | |||
16 | /** |
||
17 | * The parameter bag representing the $_GET or $_POST array |
||
18 | * @var Symfony\Component\HttpFoundation\ParameterBag |
||
19 | */ |
||
20 | private $params; |
||
21 | |||
22 | /** |
||
23 | * {@inheritdoc} |
||
24 | */ |
||
25 | 22 | public function setUp() |
|
26 | { |
||
27 | 22 | $request = $this->getRequest(); |
|
28 | |||
29 | // To prevent abuse of the automated system, we need to make sure that |
||
30 | // the IP making the request is one of the IPs we allowed in the config |
||
31 | 22 | $allowedIPs = array_map('trim', $this->container->getParameter('bzion.api.allowed_ips')); |
|
32 | 22 | $clientIP = $request->getClientIp(); |
|
33 | |||
34 | 22 | if (!$this->isDebug() && // Don't care about IPs if we're in debug mode |
|
35 | 22 | !in_array($clientIP, $allowedIPs)) { |
|
36 | // If server making the request isn't an official server, then log the unauthorized attempt and kill the script |
||
37 | |||
38 | $this->getLogger()->addNotice("Unauthorized access attempt from $clientIP"); |
||
39 | throw new ForbiddenException("Error: 403 - Forbidden"); |
||
40 | } |
||
41 | |||
42 | // We will be looking at either $_POST or $_GET depending on the status, production or development |
||
43 | 22 | $this->params = $request->request; // $_POST |
|
44 | |||
45 | 22 | if (!$this->params->has('query')) { |
|
46 | // There seems to be nothing in $_POST. If we are in debug mode |
||
47 | // however, we might have a debug request with data in $_GET |
||
48 | if ($this->isDebug() && $request->query->has('query')) { |
||
49 | $this->params = $request->query; // $_GET |
||
50 | } else { |
||
51 | throw new BadRequestException(); |
||
52 | } |
||
53 | } |
||
54 | |||
55 | // After the first major rewrite of the league overseer plugin, the |
||
56 | // API was introduced in order to provide backwards compatibility for |
||
57 | // servers that have not updated to the latest version of the plugin. |
||
58 | 22 | $this->version = $this->params->get('apiVersion', 0); |
|
59 | 22 | } |
|
60 | |||
61 | /** |
||
62 | * @ApiDoc( |
||
63 | * description="Query the LeagueOverseer API", |
||
64 | * parameters={ |
||
65 | * {"name"="query", "dataType"="string", "required"=true, "description"="query type"}, |
||
66 | * {"name"="apiVersion", "dataType"="integer", "required"=false, "description"="LeagueOverseer API version"} |
||
67 | * } |
||
68 | * ) |
||
69 | * @todo Test/improve/revoke support for API version 0 |
||
70 | */ |
||
71 | 22 | public function queryAction() |
|
72 | { |
||
73 | 22 | $matchReportQuery = $this->version == 1 ? 'reportMatch' : 'matchReport'; |
|
74 | 22 | $teamNameQuery = $this->version == 1 ? 'teamNameQuery' : 'teamName'; |
|
75 | 22 | $teamNameDumpQuery = $this->version == 1 ? 'teamDump' : 'teamInfoDump'; |
|
76 | |||
77 | 22 | switch ($this->params->get('query')) { |
|
78 | 22 | case $matchReportQuery: |
|
79 | 22 | return $this->forward('matchReport'); |
|
80 | case $teamNameQuery: |
||
81 | return $this->forward('teamName'); |
||
82 | case $teamNameDumpQuery: |
||
83 | return $this->forward('teamNameDump'); |
||
84 | default: |
||
85 | throw new BadRequestException(); |
||
86 | } |
||
87 | } |
||
88 | |||
89 | public function teamNameAction() |
||
90 | { |
||
91 | if ($this->version < 1) { |
||
92 | throw new BadRequestException(); |
||
93 | } |
||
94 | |||
95 | $param = $this->version == 1 ? 'teamPlayers' : 'bzid'; |
||
96 | |||
97 | $bzid = $this->params->get($param); |
||
98 | $team = Player::getFromBZID($bzid)->getTeam(); |
||
99 | |||
100 | $teamName = ($team->isValid()) ? preg_replace("/&[^\s]*;/", "", $team->getName()) : ''; |
||
101 | |||
102 | return new JsonResponse(array( |
||
103 | // API v1 legacy support |
||
104 | "bzid" => $bzid, // Replaced with "team_name" in API v2+ |
||
105 | "team" => $teamName, // Replaced with "player_bzid" in API v2+ |
||
106 | |||
107 | // API v2+ |
||
108 | "player_bzid" => $bzid, |
||
109 | "team_id" => $team->getId(), |
||
110 | "team_name" => $teamName, |
||
111 | )); |
||
112 | } |
||
113 | |||
114 | public function teamNameDumpAction() |
||
115 | { |
||
116 | if ($this->version < 1) { |
||
117 | throw new BadRequestException(); |
||
118 | } |
||
119 | |||
120 | // Create an array to store all teams and the BZIDs |
||
121 | $teamArray = array(); |
||
122 | |||
123 | foreach (Team::getTeams() as $team) { |
||
124 | $memberList = ""; |
||
125 | |||
126 | foreach ($team->getMembers() as $member) { |
||
0 ignored issues
–
show
|
|||
127 | $memberList .= $member->getBZID() . ","; |
||
128 | } |
||
129 | |||
130 | $teamName = preg_replace("/&[^\s]*;/", "", $team->getName()); |
||
0 ignored issues
–
show
It seems like you code against a specific sub-type and not the parent class
Model as the method getName() does only exist in the following sub-classes of Model : AliasModel , ApiKey , AvatarModel , Ban , Conversation , Country , Map , Match , News , NewsCategory , Page , Permission , Player , Role , Server , Team . Maybe you want to instanceof check for one of these explicitly?
Let’s take a look at an example: abstract class User
{
/** @return string */
abstract public function getPassword();
}
class MyUser extends User
{
public function getPassword()
{
// return something
}
public function getDisplayName()
{
// return some name.
}
}
class AuthSystem
{
public function authenticate(User $user)
{
$this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
// do something.
}
}
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types
inside the if block in such a case.
![]() |
|||
131 | $teamID = $team->getId(); |
||
132 | $teamMembers = rtrim($memberList, ","); |
||
133 | |||
134 | $teamArray[] = array( |
||
135 | // API v1 legacy support |
||
136 | "team" => $teamName, |
||
137 | "members" => $teamMembers, |
||
138 | |||
139 | // API v2+ |
||
140 | "team_id" => $teamID, |
||
141 | "team_name" => $teamName, |
||
142 | "team_members" => $teamMembers |
||
143 | ); |
||
144 | } |
||
145 | |||
146 | return new JsonResponse(array( |
||
147 | // API v1 legacy support |
||
148 | "teamDump" => &$teamArray, |
||
149 | |||
150 | // API v2+ |
||
151 | "team_list" => &$teamArray |
||
152 | )); |
||
153 | } |
||
154 | |||
155 | 22 | public function matchReportAction(Logger $log, Request $request) |
|
156 | { |
||
157 | 22 | $log->addNotice("Match data received from " . $request->getClientIp()); |
|
158 | 22 | $log->addDebug("Debug match data query: " . http_build_query($this->params->all())); |
|
159 | |||
160 | 22 | $matchType = $this->params->get('matchType', Match::OFFICIAL); |
|
161 | |||
162 | 22 | $teamOnePlayers = $this->bzidsToIdArray('teamOnePlayers'); |
|
163 | 22 | $teamTwoPlayers = $this->bzidsToIdArray('teamTwoPlayers'); |
|
164 | |||
165 | 22 | $teamOne = $teamTwo = null; |
|
166 | 22 | ||
167 | if (Match::OFFICIAL === $matchType) { |
||
168 | 22 | $teamOne = $this->getTeam($teamOnePlayers); |
|
169 | $teamTwo = $this->getTeam($teamTwoPlayers); |
||
170 | 22 | ||
171 | 21 | if ($teamOne->isValid() && $teamTwo->isValid() && $teamOne->isSameAs($teamTwo)) { |
|
0 ignored issues
–
show
It seems like
$teamTwo defined by $this->getTeam($teamTwoPlayers) on line 169 can be null ; however, Model::isSameAs() does not accept null , maybe add an additional type check?
Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass|null */
function mayReturnNull() { }
function doesNotAcceptNull(stdClass $x) { }
// With potential error.
function withoutCheck() {
$x = mayReturnNull();
doesNotAcceptNull($x); // Potential error here.
}
// Safe - Alternative 1
function withCheck1() {
$x = mayReturnNull();
if ( ! $x instanceof stdClass) {
throw new \LogicException('$x must be defined.');
}
doesNotAcceptNull($x);
}
// Safe - Alternative 2
function withCheck2() {
$x = mayReturnNull();
if ($x instanceof stdClass) {
doesNotAcceptNull($x);
}
}
![]() |
|||
172 | 21 | $log->addNotice("The '" . $teamOne->getName() . "' team played against each other in an official match. Match invalidated."); |
|
173 | throw new ForbiddenException("Holy sanity check, Batman! The same team can't play against each other in an official match."); |
||
174 | 21 | } |
|
175 | 1 | } elseif (Match::FUN === $matchType) { |
|
176 | 21 | if (count($teamOnePlayers) < 2 || count($teamTwoPlayers) < 2) { |
|
177 | throw new ForbiddenException("You are not allowed to report a match with less than 2 players per team."); |
||
178 | 1 | } |
|
179 | 1 | } |
|
180 | |||
181 | $map = Map::fetchFromAlias($this->params->get('mapPlayed')); |
||
182 | $server = Server::fetchFromAddress($this->params->get('server')); |
||
183 | |||
184 | 21 | $match = Match::enterMatch( |
|
185 | ($teamOne !== null) ? $teamOne->getId() : null, |
||
186 | 21 | ($teamTwo !== null) ? $teamTwo->getId() : null, |
|
187 | 21 | $this->params->get('teamOneWins'), |
|
188 | 21 | $this->params->get('teamTwoWins'), |
|
189 | 21 | $this->params->get('duration'), |
|
190 | 21 | null, |
|
191 | 21 | $this->params->get('matchTime'), |
|
192 | 21 | $teamOnePlayers, |
|
193 | 21 | $teamTwoPlayers, |
|
194 | 21 | $this->params->get('server'), |
|
195 | 21 | $this->params->get('replayFile'), |
|
196 | 21 | $map->getId(), |
|
197 | 21 | $this->params->get('matchType'), |
|
198 | 21 | $this->params->get('teamOneColor'), |
|
199 | 21 | $this->params->get('teamTwoColor'), |
|
200 | 21 | $this->explodeQueryParam('teamOneIPs'), |
|
201 | 21 | $this->explodeQueryParam('teamTwoIPs'), |
|
202 | $this->explodeQueryParam('teamOneCallsigns'), |
||
203 | $this->explodeQueryParam('teamTwoCallsigns') |
||
204 | 21 | ); |
|
205 | |||
206 | 21 | if ($server->isValid()) { |
|
207 | 21 | $match->setServer($server->getId()); |
|
208 | } |
||
209 | |||
210 | 21 | $log->addNotice("Match reported automatically", array( |
|
211 | 21 | 'winner' => array( |
|
212 | 'name' => $match->getWinner()->getName(), |
||
213 | 21 | 'score' => $match->getScore($match->getWinner()), |
|
214 | 21 | ), |
|
215 | 21 | 'loser' => array( |
|
216 | 'name' => $match->getLoser()->getName(), |
||
217 | 'score' => $match->getScore($match->getLoser()) |
||
218 | 21 | ), |
|
219 | 'eloDiff' => $match->getEloDiff(), |
||
220 | 21 | 'type' => $matchType, |
|
221 | 13 | 'map' => $map->getName() |
|
222 | 4 | )); |
|
223 | 9 | ||
224 | 4 | $bzfsAnnouncement = $match->getName(); |
|
225 | 4 | ||
226 | if (!$match->getWinner()->supportsMatchCount() || !$match->getLoser()->supportsMatchCount()) { |
||
227 | if ($match->getWinner()->supportsMatchCount()) { |
||
228 | $bzfsAnnouncement .= sprintf("\n %s: +%d", $match->getWinner()->getName(), $match->getEloDiff()); |
||
229 | 21 | } elseif ($match->getLoser()->supportsMatchCount()) { |
|
230 | $diff = -$match->getEloDiff(); |
||
231 | 20 | $bzfsAnnouncement .= sprintf("\n %s: %d", $match->getLoser()->getName(), $diff); |
|
232 | 20 | } |
|
233 | } |
||
234 | |||
235 | 20 | if ($match->isOfficial()) { |
|
236 | 20 | $inverseSymbol = ( |
|
237 | $match->getTeamMatchType() === Match::TEAM_V_TEAM && $match->isDraw() && |
||
238 | ($match->getWinner()->isSameAs($match->getTeamB()) || $match->getPlayerEloDiff(false) < 0) |
||
239 | ); |
||
240 | 21 | ||
241 | $symbol = ($inverseSymbol) ? '-/+' : '+/-'; |
||
242 | $bzfsAnnouncement .= sprintf("\n player elo: %s %d", $symbol, $match->getPlayerEloDiff()); |
||
243 | } |
||
244 | |||
245 | // Output the match stats that will be sent back to BZFS |
||
246 | return $bzfsAnnouncement; |
||
247 | } |
||
248 | |||
249 | /** |
||
250 | 22 | * Split a query parameter with a delimiter. |
|
251 | * |
||
252 | 22 | * @param string $parameter The query parameter to get and split |
|
253 | * @param string $delimiter A delimiter for splitting the string |
||
254 | 22 | * |
|
255 | 22 | * @return array |
|
256 | */ |
||
257 | private function explodeQueryParam($parameter, $delimiter = ',') |
||
258 | 22 | { |
|
259 | $split = explode($delimiter, $this->params->get($parameter, '')); |
||
260 | |||
261 | if (!is_array($split)) { |
||
262 | return []; |
||
263 | } |
||
264 | |||
265 | return $split; |
||
266 | } |
||
267 | 21 | ||
268 | /** |
||
269 | 21 | * Convert a comma-separated list of bzids to player IDs so we can pass them to Match::enterMatch() |
|
270 | * |
||
271 | 21 | * @param string $queryParam The query parameter storing a comma-separated list of BZIDs |
|
272 | 21 | * |
|
273 | * @return int[] A list of Player IDs |
||
274 | 21 | */ |
|
275 | 12 | private function bzidsToIdArray($queryParam) |
|
276 | 21 | { |
|
277 | 21 | $players = $this->explodeQueryParam($queryParam); |
|
278 | 17 | ||
279 | foreach ($players as &$player) { |
||
280 | 21 | $player = Player::getFromBZID($player)->getId(); |
|
281 | } |
||
282 | |||
283 | return $players; |
||
284 | 17 | } |
|
285 | |||
286 | /** |
||
287 | * Queries the database to get the team which a conversation of players belong to |
||
288 | * |
||
289 | * @param int[] $players The IDs of players |
||
290 | 22 | * @return Team The team |
|
291 | */ |
||
292 | 22 | private function getTeam($players) |
|
293 | { |
||
294 | $team = null; |
||
295 | |||
296 | foreach ($players as $id) { |
||
297 | $player = Player::get($id); |
||
298 | |||
299 | if ($player->isTeamless()) { |
||
300 | return Team::invalid(); |
||
301 | } elseif ($team == null) { |
||
302 | $team = $player->getTeam(); |
||
303 | } elseif ($team->getId() != $player->getTeam()->getId()) { |
||
304 | // This player is on a different team from the previous player! |
||
305 | return Team::invalid(); |
||
306 | } |
||
307 | } |
||
308 | |||
309 | return $team; |
||
310 | } |
||
311 | |||
312 | /** |
||
313 | * {@inheritdoc} |
||
314 | */ |
||
315 | protected static function getLogChannel() |
||
316 | { |
||
317 | return 'api'; |
||
318 | } |
||
319 | } |
||
320 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the parent class: