Skip to content

Commit

Permalink
Install PII data protections with redundancy (#615)
Browse files Browse the repository at this point in the history
  • Loading branch information
dqwiki authored Feb 1, 2023
1 parent ee612e9 commit a053f48
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
24 changes: 24 additions & 0 deletions app/Http/Controllers/Appeal/PublicAppealController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
use Illuminate\Http\Request;
use Illuminate\Support\Facades\DB;
use Illuminate\Validation\Rule;
use App\Utils\IPUtils;
use Redirect;

class PublicAppealController extends Controller
{
Expand All @@ -36,6 +38,28 @@ public function store(Request $request)
'hiddenip' => 'nullable|ip'
]);

//If blocktype == 0 and appealfor not IP/range
if ($data['blocktype']==0 && !(IPUtils::isIp($data['appealfor']) || IPUtils::isIpRange($data['appealfor']))) {
return Redirect::back()->withErrors(['msg'=>'That is not a valid IP address, please try again.'])->withInput();
}

if ($data['blocktype']!=0 && (IPUtils::isIp($data['appealfor']) || IPUtils::isIpRange($data['appealfor']))) {
return Redirect::back()->withErrors(['msg'=>'You need to enter a username, not an IP address, please try again.'])->withInput();
}

if ($data['blocktype']==2 && (!isset($data['hiddenip'])||$data['hiddenip']===NULL)) {
return Redirect::back()->withErrors(['msg'=>'No underlying IP address provided, please try again.'])->withInput();

}

if ($data['blocktype']==2 && (!isset($data['hiddenip'])||$data['hiddenip']==NULL)) {
if (!(IPUtils::isIp($data['hiddenip']) || IPUtils::isIpRange($data['hiddenip']))) {
return Redirect::back()->withErrors(['msg'=>'The underlying IP is not an IP address, please try again.'])->withInput();
}
}



// back compat, at least for now
$data['wiki'] = Wiki::where('id', $data['wiki_id'])->firstOrFail()->database_name;

Expand Down
14 changes: 11 additions & 3 deletions app/Jobs/GetBlockDetailsJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ public function __construct(Appeal $appeal)
* Utility method to check if block target given by user needs correcting
* @param string $givenBlockTarget Block target given by the blocked user in the form
* @param string $actualBlockTarget Block target queried from MediaWiki API
* @param int $blocktype Whether this is an IP, User, or IP under user taken from the appeal
* @return bool true if block target should be corrected in the database, false otherwise
*/
private function shouldCorrectBlockTarget(string $givenBlockTarget, string $actualBlockTarget)
private function shouldCorrectBlockTarget(string $givenBlockTarget, string $actualBlockTarget, int $blocktype)
{
// if it's already correct, no need to do anything
if (strtolower($givenBlockTarget) === strtolower($actualBlockTarget)) {
Expand All @@ -49,6 +50,13 @@ private function shouldCorrectBlockTarget(string $givenBlockTarget, string $actu
return false;
}

// Stopping ANY Account based appeal from being corrected down to an IP address
// BE VERY CAREFUL WITH THIS LINE, THIS HAS CAUSED A CVE BEFORE.
// https://github.com/UTRS2/utrs/security/advisories/GHSA-h76p-r6xc-7rwx
if($blocktype!=0) {
return false;
}

return true;
}

Expand All @@ -61,7 +69,7 @@ public function handleBlockData(Block $block)
{
$status = Appeal::STATUS_OPEN;

/*if ($block && $this->shouldCorrectBlockTarget($this->appeal->appealfor, $block->getBlockTarget())) {
if ($block && $this->shouldCorrectBlockTarget($this->appeal->appealfor, $block->getBlockTarget(),$this->appeal->blocktype)) {
$this->appeal->appealfor = $block->getBlockTarget();

$duplicateAppeal = Appeal::where('appealfor', $this->appeal->appealfor)
Expand Down Expand Up @@ -104,7 +112,7 @@ public function handleBlockData(Block $block)
'protected' => 0
]);
}
}*/
}

$this->appeal->update([
'blockfound' => 1,
Expand Down

0 comments on commit a053f48

Please sign in to comment.