Fix the server-executable file security checks.

Review Request #880 — Created Sept. 21, 2017 and updated

Review Board
We have a security check that ensures the server isn't able to execute
uploaded file attachments. A change a while back regressed the checks,
due to usage of `build_server_url()`. The check was passing in multiple
path components to this method, which then get forwarded to `urljoin()`.
However, `urljoin()` only accepts two components in total (one of which
is the root URL of the server). This meant that the filename being
checked was being passed as the `allow_fragments` keyword argument
instead, and the URL being downloaded was actually the root
`media/uploaded/files/` path, causing the checks to always fail.

This fixes the function signature and docs to `build_server_url()` to
ensure that it will always accept only a single path, and fixes the
check to pass the correct path to the file.

Testing Done:
Unit tests pass.

Verified that the paths being built for the security checks were correct,
and that the security checks were providing the correct results.

Reviewed at

  • 1
  • 0
  • 0
  • 0
  • 1
Description From Last Updated
Test comment guest6920 guest6920
  2. reviewboard/admin/ (Diff revision 1)

    Test comment