@@ -55,6 +55,27 @@ pub fn expand_path(user: Option<&expand_path::ForUser>, path: &BStr) -> Result<P
5555 } )
5656}
5757
58+ /// Classification of a portion of a URL by whether it is *syntactically* safe to pass as an argument to a command-line program.
59+ ///
60+ /// Various parts of URLs can be specified to begin with `-`. If they are used as options to a command-line application
61+ /// such as an SSH client, they will be treated as options rather than as non-option arguments as the developer intended.
62+ /// This is a security risk, because URLs are not always trusted and can often be composed or influenced by an attacker.
63+ /// See <https://secure.phabricator.com/T12961> for details.
64+ ///
65+ /// # Security Warning
66+ ///
67+ /// This type only expresses known *syntactic* risk. It does not cover other risks, such as passing a personal access
68+ /// token as a username rather than a password in an application that logs usernames.
69+ #[ derive( Debug , PartialEq , Eq , Copy , Clone ) ]
70+ pub enum ArgumentSafety < ' a > {
71+ /// May be safe. There is nothing to pass, so there is nothing dangerous.
72+ Absent ,
73+ /// May be safe. The argument does not begin with a `-` and so will not be confused as an option.
74+ Usable ( & ' a str ) ,
75+ /// Dangerous! Begins with `-` and could be treated as an option. Use the value in error messages only.
76+ Dangerous ( & ' a str ) ,
77+ }
78+
5879/// A URL with support for specialized git related capabilities.
5980///
6081/// Additionally there is support for [deserialization](Url::from_bytes()) and [serialization](Url::to_bstring()).
@@ -85,12 +106,12 @@ pub struct Url {
85106pub port : Option < u16 > ,
86107 /// The path portion of the URL, usually the location of the git repository.
87108///
88- /// # Security- Warning
109+ /// # Security Warning
89110///
90111/// URLs allow paths to start with `-` which makes it possible to mask command-line arguments as path which then leads to
91112/// the invocation of programs from an attacker controlled URL. See <https://secure.phabricator.com/T12961> for details.
92113///
93- /// If this value is going to be used in a command-line application, call [Self::path_argument_safe()] instead.
114+ /// If this value is ever going to be passed to a command-line application, call [Self::path_argument_safe()] instead.
94115pub path : BString ,
95116}
96117
@@ -164,48 +185,101 @@ impl Url {
164185
165186/// Access
166187impl Url {
167- /// Returns the user mentioned in the url, if present.
188+ /// Return the username mentioned in the URL, if present.
189+ ///
190+ /// # Security Warning
191+ ///
192+ /// URLs allow usernames to start with `-` which makes it possible to mask command-line arguments as username which then leads to
193+ /// the invocation of programs from an attacker controlled URL. See <https://secure.phabricator.com/T12961> for details.
194+ ///
195+ /// If this value is ever going to be passed to a command-line application, call [Self::user_argument_safe()] instead.
168196pub fn user ( & self ) -> Option < & str > {
169197 self . user . as_deref ( )
170198 }
171- /// Returns the password mentioned in the url, if present.
199+
200+ /// Classify the username of this URL by whether it is safe to pass as a command-line argument.
201+ ///
202+ /// Use this method instead of [Self::user()] if the host is going to be passed to a command-line application.
203+ /// If the unsafe and absent cases need not be distinguished, [Self::user_argument_safe()] may also be used.
204+ pub fn user_as_argument ( & self ) -> ArgumentSafety < ' _ > {
205+ match self . user ( ) {
206+ Some ( user) if looks_like_command_line_option ( user. as_bytes ( ) ) => ArgumentSafety :: Dangerous ( user) ,
207+ Some ( user) => ArgumentSafety :: Usable ( user) ,
208+ None => ArgumentSafety :: Absent ,
209+ }
210+ }
211+
212+ /// Return the username of this URL if present *and* if it can't be mistaken for a command-line argument.
213+ ///
214+ /// Use this method or [Self::user_as_argument()] instead of [Self::user()] if the host is going to be
215+ /// passed to a command-line application. Prefer [Self::user_as_argument()] unless the unsafe and absent
216+ /// cases need not be distinguished from each other.
217+ pub fn user_argument_safe ( & self ) -> Option < & str > {
218+ match self . user_as_argument ( ) {
219+ ArgumentSafety :: Usable ( user) => Some ( user) ,
220+ _ => None ,
221+ }
222+ }
223+
224+ /// Return the password mentioned in the url, if present.
172225pub fn password ( & self ) -> Option < & str > {
173226 self . password . as_deref ( )
174227 }
175- /// Returns the host mentioned in the url, if present.
228+
229+ /// Return the host mentioned in the URL, if present.
176230///
177- /// # Security- Warning
231+ /// # Security Warning
178232///
179233/// URLs allow hosts to start with `-` which makes it possible to mask command-line arguments as host which then leads to
180234/// the invocation of programs from an attacker controlled URL. See <https://secure.phabricator.com/T12961> for details.
181235///
182- /// If this value is going to be used in a command-line application, call [Self::host_argument_safe()] instead.
236+ /// If this value is ever going to be passed to a command-line application, call [Self::host_as_argument()]
237+ /// or [Self::host_argument_safe()] instead.
183238pub fn host ( & self ) -> Option < & str > {
184239 self . host . as_deref ( )
185240 }
186241
242+ /// Classify the host of this URL by whether it is safe to pass as a command-line argument.
243+ ///
244+ /// Use this method instead of [Self::host()] if the host is going to be passed to a command-line application.
245+ /// If the unsafe and absent cases need not be distinguished, [Self::host_argument_safe()] may also be used.
246+ pub fn host_as_argument ( & self ) -> ArgumentSafety < ' _ > {
247+ match self . host ( ) {
248+ Some ( host) if looks_like_command_line_option ( host. as_bytes ( ) ) => ArgumentSafety :: Dangerous ( host) ,
249+ Some ( host) => ArgumentSafety :: Usable ( host) ,
250+ None => ArgumentSafety :: Absent ,
251+ }
252+ }
253+
187254 /// Return the host of this URL if present *and* if it can't be mistaken for a command-line argument.
188255///
189- /// Use this method if the host is going to be passed to a command-line application.
256+ /// Use this method or [Self::host_as_argument()] instead of [Self::host()] if the host is going to be
257+ /// passed to a command-line application. Prefer [Self::host_as_argument()] unless the unsafe and absent
258+ /// cases need not be distinguished from each other.
190259pub fn host_argument_safe ( & self ) -> Option < & str > {
191- self . host ( ) . filter ( |host| !looks_like_argument ( host. as_bytes ( ) ) )
260+ match self . host_as_argument ( ) {
261+ ArgumentSafety :: Usable ( host) => Some ( host) ,
262+ _ => None ,
263+ }
192264 }
193265
194- /// Return the path of this URL *and* if it can't be mistaken for a command-line argument.
266+ /// Return the path of this URL *if* it can't be mistaken for a command-line argument.
195267/// Note that it always begins with a slash, which is ignored for this comparison.
196268///
197- /// Use this method if the path is going to be passed to a command-line application.
269+ /// Use this method instead of accessing [Self::path] directly if the path is going to be passed to a
270+ /// command-line application, unless it is certain that the leading `/` will always be included.
198271pub fn path_argument_safe ( & self ) -> Option < & BStr > {
199272 self . path
200273 . get ( 1 ..)
201- . and_then ( |truncated| ( !looks_like_argument ( truncated) ) . then_some ( self . path . as_ref ( ) ) )
274+ . and_then ( |truncated| ( !looks_like_command_line_option ( truncated) ) . then_some ( self . path . as_ref ( ) ) )
202275 }
203276
204- /// Returns true if the path portion of the url is `/`.
277+ /// Return true if the path portion of the URL is `/`.
205278pub fn path_is_root ( & self ) -> bool {
206279 self . path == "/"
207280 }
208- /// Returns the actual or default port for use according to the url scheme.
281+
282+ /// Return the actual or default port for use according to the URL scheme.
209283/// Note that there may be no default port either.
210284pub fn port_or_default ( & self ) -> Option < u16 > {
211285 self . port . or_else ( || {
@@ -221,13 +295,13 @@ impl Url {
221295 }
222296}
223297
224- fn looks_like_argument ( b : & [ u8 ] ) -> bool {
298+ fn looks_like_command_line_option ( b : & [ u8 ] ) -> bool {
225299 b. first ( ) == Some ( & b'-' )
226300}
227301
228302/// Transformation
229303impl Url {
230- /// Turn a file url like `file://relative` into `file:///root/relative`, hence it assures the url 's path component is absolute, using
304+ /// Turn a file URL like `file://relative` into `file:///root/relative`, hence it assures the URL 's path component is absolute, using
231305/// `current_dir` if necessary.
232306pub fn canonicalized ( & self , current_dir : & std:: path:: Path ) -> Result < Self , gix_path:: realpath:: Error > {
233307 let mut res = self . clone ( ) ;
@@ -287,7 +361,7 @@ impl Url {
287361
288362/// Deserialization
289363impl Url {
290- /// Parse a URL from `bytes`
364+ /// Parse a URL from `bytes`.
291365pub fn from_bytes ( bytes : & BStr ) -> Result < Self , parse:: Error > {
292366 parse ( bytes)
293367 }
0 commit comments