Skip to content

Add limited broadcast addr to local network block list in UrlBlocker

Nick Malcolm requested to merge nmalcolm-337796-block-reserved-ip into master

What does this MR do and why?

Fulfils 255.255.255.255 IP address should be blocked (#337796 - closed).

UrlBlocker protects GitLab and its users from attacks such as Server Side Request Forgery and DNS Rebind attacks.

255.255.255.255 is the "limited broadcast address", which is used to make requests to all hosts on a local physical network 1. Properly configured routers won't route it. Historically it was used to wake up offline PCs on a LAN which, since they were asleep, didn't have IP addresses 2.

Until now, setting allow_local_network had no effect on blocking 255.255.255.255, whether true or false. Now, when allow_local_network is set to false 255.255.255.255 is blocked through the introduction of a check named validate_limited_broadcast_address.

While UrlBLocker defaults allow_local_network to true, in practice it is almost always false because of a convention to use the GitLab configuration option which defaults to false.

If a GitLab administrator still wants to reach 255.255.255.255, it can be added explicitly in the Allow List 3.

There is no reason a GitLab user would want to reach this, but it could potentially be misused if an attacker finds a component vulnerable to DNS rebinding, for example.

This commit aims to fulfil #337796 (closed)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

  • I have evaluated the MR acceptance checklist for this MR. Yes, except for:
    • I have considered using a feature flag for this change because the change may be high risk.
      • This does not need a feature flag because allow_local_network is set to true by default in UrlBlocker and so this added blocking condition won't be checked.
      • When allow_local_network is set to false, an admin would expect 255.255.255.255 to be blocked, and so this shouldn't need a feature flag.
    • I have informed the Infrastructure department of a default setting or new setting change per definition of done, or decided that this is unnecessary.
      • This is unnecessary because the defaults are not changing

Follo

Edited by Nick Malcolm

Merge request reports