Closed Bug 1194010 Opened 5 years ago Closed 2 years ago

ICE TCP might connect to any port


(Core :: WebRTC: Networking, defect, P4)




Tracking Status
firefox43 --- affected
firefox65 --- fixed
Blocking Flags:


(Reporter: drno, Assigned: iidebyo)


(Blocks 1 open bug)


(Whiteboard: [good first bug])


(1 file)

We should consider re-using the port blacklist from here for ICE TCP.
One argument is that we are just going to waste resources trying to connect to these. The other argument might be to not crash services on these ports.
Yes, this seems like a good idea.
Whiteboard: [good first bug]
backlog: --- → webrtc/webaudio+
Rank: 37
Priority: -- → P3
Hi, would like to work on this bug, please assign it to me, can you also provide more info.Thank you :)
Flags: needinfo?(drno)
So if you look at this code:
That is a list of port numbers which Firefox HTTP client should not connect to. The list is being converted into an internal array here:
And then the actual checking seems to happen here:

Right now you can ask our ICE TCP implementation as well to connect to any port number. As we seem to know from experience that it is not a good idea to connect to the ports in |gBadPortList| it would be good if we could re-use that list and prevent ICE TCP from making connections to these ports as well.

I think we try to re-use the |gBadPortList| list. So probably you want to move that list into a header file to make it accessible from the nICEr code. From looking at the code in the AllowPort() function it looks like in case of the HTTP client this list can be over-written by the URL scheme. As ICE does not use URLs I would simply write our own little function for nICEr which simply checks if a given port number is on the blacklist or not.
Theoretically you could also introduce a IsOnBackList() function in the netwerk code, which then gets internally called by the existing AllowPort() function and by nICEr. Then AllowPort() would become a wrapper around your new function which also allows a URL scheme to overwrite the blacklisting, where nICEr simply just calls only your new blacklisting function directly without the option to over-write the result.

Let me know if this make it clear what is need, or if you have further question.
Assignee: nobody → bmanojkumar24
Flags: needinfo?(drno)
Hi, thank you for the detailed explanation.I got few questions, in which header file should i write the function "IsOnBlackList()" and where should i call this function in "nICEr" code, I understood "AllowPort()" should call "IsOnBlackList()", apart from URL scheme.Thank you :)
Flags: needinfo?(drno)
Mass change P3->P4 to align with new Mozilla triage process.
Priority: P3 → P4
Port restriction is delegated to the io service.  Port 0 is explicitly

NS_CheckPortSafety emits a warning which pollutes the gtests a bit.  It is only
tested if the port is not 0.
Assignee: bmanojkumar24 → paul.m.vitale
Keywords: checkin-needed
Flags: needinfo?(drno)
Pushed by
Block webrtc socket creation on restricted ports r=drno
Keywords: checkin-needed
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.