You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Many interfaces such as Listeners::add_tcp take a &str for the IP address, and rely on into/try_into conversions (sometimes unwrap-ing, such as BasicPeer::new) to convert to pingora-core::SocketAddr or std::net::SocketAddr.
Ideally we would not unwrap: this means that river will want to pre-validate the IP address to avoid runtime panics. However, it might be better to explicitly take the intended internal format, e.g. pingora-core::SocketAddr, or T: Into<pingora-core::SocketAddr>, to avoid cases where frontends like river use different parsing or validation logic to pingora, which may lead to bugs.
Security bugs have happened in the past (at least for URLs/URIs and paths, not sure about IP Addresses) where different components used different validation/parsing rules, e.g. different levels of allowed escaping, even if this is not exploitable it might cause unexpected panics if the config has an ip address format that river allows but pingora rejects.
Additional context
This is similar to #182, basically everywhere "stringly typed data" (that isn't actually a text string) is used, it should be pushed out to the edge and validated immediately, potentially removing it entirely from pingora/pingora-*, and handled ONLY in the frontend/configuration handling crates.
The text was updated successfully, but these errors were encountered:
Describe the bug
Many interfaces such as
Listeners::add_tcp
take a&str
for the IP address, and rely oninto
/try_into
conversions (sometimesunwrap
-ing, such asBasicPeer::new
) to convert topingora-core::SocketAddr
orstd::net::SocketAddr
.Ideally we would not
unwrap
: this means thatriver
will want to pre-validate the IP address to avoid runtime panics. However, it might be better to explicitly take the intended internal format, e.g.pingora-core::SocketAddr
, orT: Into<pingora-core::SocketAddr>
, to avoid cases where frontends likeriver
use different parsing or validation logic topingora
, which may lead to bugs.Security bugs have happened in the past (at least for URLs/URIs and paths, not sure about IP Addresses) where different components used different validation/parsing rules, e.g. different levels of allowed escaping, even if this is not exploitable it might cause unexpected panics if the config has an ip address format that river allows but pingora rejects.
Additional context
This is similar to #182, basically everywhere "stringly typed data" (that isn't actually a text string) is used, it should be pushed out to the edge and validated immediately, potentially removing it entirely from
pingora
/pingora-*
, and handled ONLY in the frontend/configuration handling crates.The text was updated successfully, but these errors were encountered: