search

ExtremeFiretop / MerlinAutoUpdate-Router

Merlin(A)uto(U)pdate is a Merlin router script which allows you to remotely identify a stable firmware update for an ASUS Merlin router, and automatically download and update via an unattended method directly from the router.
https://www.snbforums.com/threads/merlinau-v1-2-7-the-ultimate-firmware-auto-updater-amtm-addon.91326/
GNU General Public License v3.0
21 stars 1 forks source link

Improvements when detecting WebGUI access rules #294

Closed Martinski4GitHub closed 3 months ago

Martinski4GitHub commented 3 months ago

Made code improvements to parse correctly the following use cases of WebGUI access rules:

1) Using CIDR notation for a single LAN IP address (e.g. 192.168.100.1/32) 2) Using CIDR notation for a block of LAN IP addresses (e.g. 192.168.100.0/28) using the network address.

This should take care of 99% of the valid use cases.

Martinski4GitHub commented 3 months ago

@ExtremeFiretop, Please review & test whenever you have the time. So far, it's working well with my use cases.

ExtremeFiretop commented 3 months ago
  1. Using CIDR notation for a block of LAN IP addresses (e.g. 192.168.100.0/28) using the network address.

This should take care of 99% of the valid use cases.

This is very good, a great catch, I honestly didn't even notice you could enter a network address range:

image

Only after reviewing this PR did I go... Why would we need to check a range? Logged in and sure enough saw you could approve a range, which means my method wouldn't catch a router that has access to itself just due to falling within that range that's been allowed.

Because the feature is limited to only 4 "entries"; I didn't expect a range to be allowed, I expected a 4 IP hard limit.

Approved and merged!

Martinski4GitHub commented 3 months ago
  1. Using CIDR notation for a block of LAN IP addresses (e.g. 192.168.100.0/28) using the network address.

This should take care of 99% of the valid use cases.

This is very good, a great catch, I honestly didn't even notice you could enter a network address range:

image

Only after reviewing this PR did I go... Why would we need to check a range? Logged in and sure enough saw you could approve a range, which means my method wouldn't catch a router that has access to itself just due to falling within that range that's been allowed.

Because the feature is limited to only 4 "entries"; I didn't expect a range to be allowed, I expected a 4 IP hard limit.

Approved and merged!

As the saying goes: "The devil is in the details." Late last night, while testing the latest code for RMerlin F/W updates, I realized there are 2 more use cases that should be considered. Granted, they're not very common, but I know of one router installation where the network IP address is not the usual default. See the new PR #295 that I just submitted.