Re: 3 interesting bugs

Inline :)

On Tue, Sep 27, 2016 at 9: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.
>

An allowed list of directories is a reasonable idea. The way that the way
that the existing local ends work is to add a "file detector
<https://github.com/SeleniumHQ/selenium/blob/master/java/client/src/org/openqa/selenium/remote/LocalFileDetector.java>"
to send keys. When they detect that a file is being uploaded (by the
somewhat crude method of comparing text with file paths) they cause that
file to be uploaded to the remote server. The remote server places the file
in a known location, preserving the file name, and then modifies the value
used with send keys to reflect this new location. It would be easy enough
to make this work with an allowed list.

The only problem would occur when the local end isn't using an intermediate
remote server, but talking directly to (e.g.) the safaridriver service.
That could be mitigated by always copying files to an allowed location
before sending the keyboard input.


> 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?
>

My personal opinion is that I'm not too concerned, but if we are, then a
block or allow list could be made into a workable solution.


> Certainly it is true that file uploads punch a whole in the sandboxing
> that is given by the web platform.


True :)


>
> > 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.
>

The average user is normally just uploading test data files from within
their test source tree. If the allow list contains the current working
directory and $TEMP, most people won't notice.


> 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.
>

It's implementation specific. I'd suggest that implementations sniff the
element type and check that the input is allowable before proceeding, using
whatever is most appropriate (an OS level dialog if necessary, or just
spamming in values)


> 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.


Mainly because we don't know what'll happen when a user clicks. To be fair,
we occasionally get requests about this on the selenium user mailing lists,
typically when someone wants to print a document as part of their test
(don't ask) or download a file. The advice we always give is "don't do
that", and it seems to be heeded.


> > 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.
>
>
Cheers,

Simon