Bug #90047
closed
BUG with IP6 in IpLocker.php after protokoll switch ipv4 to ipv6
Added by Timo Poppinga over 4 years ago.
Updated over 4 years ago.
Description
IpLocker.php dose not work with
TYPO3\CMS\Core\Utility\MathUtility\IpLocker
wrong code
public function validateRemoteAddressAgainstSessionIpLock(string $ipAddress, string $sessionIpLock): bool
{
if ($sessionIpLock === static::DISABLED_LOCK_VALUE) {
return true;
}
$ipToCompare = $this->isIpv6Address($sessionIpLock)
? $this->getIpLockPartForIpv6Address($ipAddress)
: $this->getIpLockPartForIpv4Address($ipAddress);
return $ipToCompare === $sessionIpLock;
}
Can only be commperd if same protokoll is used
to fix:
$ipToCompare = $this->isIpv6Address($ipAddress) !!!!!
? $this->getIpLockPartForIpv6Address($ipAddress)
: $this->getIpLockPartForIpv4Address($ipAddress);
But then user will be logged out at protocol switch
- Status changed from New to Needs Feedback
- Target version deleted (
next-patchlevel)
With a protocol switch you mean switching from v4 to v6, right?
If so, we suggest disabling the IPlocking completely, since it has no benefit.
See also #89869
- Related to Bug #89869: IP Lock feature broken by modern IPv6 - Should be disabled by default or refactored added
Hi Markus, yes that is what I mean "With a protocol switch you mean switching from v4 to v6". I do alredy disable IP-Lock witch is working fine.
My Issue is an hint if some one doese not disable IP-Lock an warning is thrown witch is in an good server setup an error. By changing the code no warning appiers but you are right user will still be locked out. Thx
If I understand you correctly, I can close this ticket, right?
yes and no
point is this code doese not work well on default setting iplock = on:
$ipToCompare = $this->isIpv6Address($sessionIpLock)
? $this->getIpLockPartForIpv6Address($ipAddress)
: $this->getIpLockPartForIpv4Address($ipAddress);
return $ipToCompare === $sessionIpLock;
}
if ip swichtes the condition checking choosing the normalizer checks on $sessionIpLock but then the $ipAddress is send to the methoed. If for example session is locked witch ip4 and then connection is swicht to ipv6 $this->getIpLockPartForIpv4Address tryes to prosess an ipv6 witch triggerd an warning in most context this throws an php error.
I see three options:
1. accept it and update documentation to do not use iplock in mixed ip stack
2. make some more checks in getIpLockPartFor Ipv4Address getIpLockPartFor Ipv6 Address so that no wornings are thrown if ip stack is swicht
3. Use my code above (my is has security impact)
4.
My favorite one is the throw a special exception if ipLook is on and and ip stack change is detected
like:
if($this->isIpv6Address($sessionIpLock) !== $this->isIpv6Address($ipAddress)) {
throw new \Exception('You are using both ip stacks ipv6 and ipv4 ip lock does not work please disable it in the install tool');
}
- Status changed from Needs Feedback to Accepted
Alright.
I don't think it is good to introduce a new exception that might break one's Frontend.
I would rather stick to logging a critical-message.
I would appreciate implementing your fix to prevent PHP errors as a quick and easy patch.
The logging part should be an improvement for integrators and I would put that into a dedicated change.
Can you provide the change you proposed to our review system?
- Status changed from Accepted to Under Review
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
- Status changed from Resolved to Closed
Also available in: Atom
PDF