Re: 3 interesting bugs

On Tue, Oct 4, 2016 at 6:07 PM, Brian Burg <bburg@apple.com> wrote:

>
> > On Sep 27, 2016, at 1:03 PM, Andreas Tolfsen <ato@mozilla.com> wrote:
> >
> > Olá Brian,
> >
> > Your email had many useful comments.
> >
> > Brian Burg <bburg@apple.com> writes:
> >
> >>> On Sep 27, 2016, at 6:11 AM, Andreas Tolfsen <ato@mozilla.com>
> >>> wrote:
> >>>
> >>> The brief summary is that sending keys to <input type=file> should
> >>> trigger implementation-specific steps to ensure the first element of
> >>> the `files` property is replaced with a new File object.  If sending
> >>> keys to <input type=file multiple>, another File object should be
> >>> appended to the `files` property.
> >>
> >> Currently, safaridriver has no support for <input type=file>. I
> >> looked into how we might do it, and am worried about it being a
> >> vector for session hijacking or exfiltration of user data stored at
> >> well-known filesystem locations. Have any other implementors added
> >> restrictions to mitigate this? One thing I was thinking about is, at
> >> session creation time, having a capability key to set the root
> >> directory from which files can be uploaded.
> >
> > That’s an interesting idea.  It’s certainly possible to impose such a
> > whitelist restriction in the remote end.  It’s also not too different
> > from the idea we rejected at the F2F with regards to a wildcard
> > whitelist of SSL cert ignore domains.
> >
> > The question is if we expect our users to understand the subtleties of
> > such a restriction, and whether we think it is a strong security risk
> > considering the attacker already has access to the WebDriver session?
>
> From a Safari point of view, yes, it's a big risk. Our web server is
> tightly sandboxed, as are the rendering processes where JS and DOM run.
> Without mitigations, the file upload command would be a huge hole in the
> sandbox. If some otherwise-sandboxed local process on the machine became
> compromised, it could send an HTTP request to escalate file read privileges
> and exfiltrate sensitive user files.


So I'm hearing an argument for a vendor-defined allow list of directories
that can contain files to upload? If you're worried about people
maliciously abusing file upload, it's probably best not to set this as a
capability….


> > Certainly it is true that file uploads punch a whole in the sandboxing
> > that is given by the web platform.
> >
> >> File opening would of course be subject to any OS-level file modes or
> >> ACLs.
> >
> > Yes, let’s be clear that it depends on the file system rights on the
> > operating system.  However, I don’t think the average user will know
> > the technicalities of the filesystem.
> >
> > Also it only provides protection on typical Unix filesystems.
> >
> >> I also really dislike shoehorning this into sendKeys for obvious
> >> reasons, but I think that ship has sailed for v1.
> >
> > I don’t think anyone is a fan of this, but the appending shoehorning is
> > surprisingly effective.  The idea is that one should be able to call
> > Element Clear to reset the `files` array.
> >
> >>>> Bud #2:
> >>>> https://developer.microsoft.com/en-us/microsoft-edge/
> platform/issues/8515651/
> >>>> This covers an issue I brought up at the July F2F around the
> >>>> various oddball inputs. We have issues "sending keys" to these as
> >>>> they are somewhat special, so thoughts on this would be
> >>>> interesting. I'm guessing value should just be set and any
> >>>> relevant events fired but wanted to know thoughts.
> >>>
> >>> We covered this at our F2F in Boston.  Instead of sending key
> >>> strokes as we do for <input type=text> and <textarea>, we will
> >>> treat all HTML
> >>> (5) input elements such as <input type=color> et al. by setting
> >>> their `value` attributes directly to the supplied string.
> >>>
> >>> This means sending "#FFB6C1" to <input type=color> will cause its
> >>> internal colour state to change to pink.  Or to put it more simply,
> >>> we delegate to what the HTML spec tells the browser to do when
> >>> setting the property.
> >>>
> >>> Again, this is also uncovered in the current specification draft.
> >>> As above I will fix that this week.
> >>
> >> This seems like a reasonable way to change the values. It's unclear
> >> what would happen if the automation command were to click on, say, a
> >> color picker widget. Should it just be suppressed from presenting,
> >> since it might be an OS-level dialog?
> >
> > This is a good question.  I suspect this will currently cause the
> > browser to go into an unrecoverable state, much like what Clay
> > described initially.
> >
> > I’m not sure we can disallow actions API clicking as we don’t know what
> > it will hit?  Currently this is completely undefined by the spec.
>
> I don't think we can disallow certain user actions, but we could say that
> they must run to completion without further user interaction. This would
> mean browsers need to take care that the session isn't hung by IME dialogs,
> context menus, permissions dialogs, or anything else that can halt JS
> execution indefinitely.


The alternative is to say that if a user action requires further user
interaction, the session is assumed to be in an unusable state and further
interaction through the webdriver protocol will have undefined effects.


> >> safaridriver keeps track of the "WebDriver" focused browsing context
> >> separately from the UI state. However, it does cause Safari to take
> >> focus when simulating user inputs. This is a restriction of the
> >> windowing system and the fact that safaridriver sends simulated
> >> OS-level events to the browser application, which do nothing if the
> >> correct application window isn't focused. This focusing is done
> >> automatically so users don't need to worry about it.
> >
> > This sounds fine to me.
> >
>

Simon