Project

General

Profile

Bug #7163

IGMP Proxy does not valid inputs

Added by Phillip Davis almost 4 years ago. Updated 8 months ago.

Status:
Resolved
Priority:
Normal
Category:
IGMP Proxy
Target version:
Start date:
01/25/2017
Due date:
% Done:

100%

Estimated time:
Affected Version:
All
Affected Architecture:

Description

IGMP Proxy Edit

Threshold: no validation is done, I can put "abc" "-42"... - I think it must be a positive integer, 1 to some max like 255.

Networks: no validation is done, I can put "w.x.y.z" - IPv4 networks should be allowed, I don;t think IPv6 addresses should be allowed.
The Networks field has setPattern('[a-zA-Z0-9_.:]+') - why is that? Are host/domain names somehow possible here?

Associated revisions

Revision 8d656a00 (diff)
Added by Viktor Gurov 8 months ago

IGMP Proxy WebGUI input validation. Issue #7163

History

#1 Updated by Phillip Davis almost 4 years ago

If someone confirms what the validation requirements are, I can make it so.

#2 Updated by Jim Thompson almost 4 years ago

  • Assignee set to Jim Pingle

#3 Updated by Jim Pingle over 3 years ago

  • Assignee changed from Jim Pingle to Phillip Davis

In the config for igmpproxy, Network populates altnet and has to be in subnet format. Since the GUI has a drop-down for the CIDR/Prefix, that means Networks should validate an IP address only. As far as I can tell it can't use hostnames or aliases. The GUI code reassembles the value into subnet style before storing it in a space-separated list. So it should be tested before it gets that far.

"threshold" has no limits in the igmpproxy source or documentation but given that it is compared to a TTL, I'd say it should be limited to nearly the same range as a TTL, allowing for it to be set lower or higher than the possible values for a TTL.

Packets with a lower TTL than the threshol[d] value will be ignored. This setting is optional, and by default the threshold is 1.

Given that, perhaps -1-256 could be the allowed range. -1 would ignore nothing, 256 would ignore everything (which doesn't seem to make sense, but...)

Feel free to take a shot at the validation, Phil. If you don't have time right now then I can do it.

#5 Updated by Jim Pingle 8 months ago

  • Status changed from New to Pull Request Review
  • Target version set to 2.5.0

#6 Updated by Renato Botelho 8 months ago

  • Status changed from Pull Request Review to Feedback
  • Assignee changed from Phillip Davis to Renato Botelho
  • % Done changed from 0 to 100

PR has been merged. Thanks!

#7 Updated by Viktor Gurov 8 months ago

  • Status changed from Feedback to Resolved

works as expected on 2.5.0.a.20200220.1948

Also available in: Atom PDF