AssertionError if [inherit] htdocs-dir is empty or not defined

[Ryan J Ollos]

This was initially posted in comment:2:ticket:11294.

If [inherit] htdocs_dir is empty or doesn't exist, then the following traceback can be seen in the console when we try to load a page that uses an image from shared:

02:45:12 PM Trac[main] ERROR: Internal Server Error: 
Traceback (most recent call last):
  File "/home/user/Workspace/t11294/teo-rjollos.git/trac/web/main.py", line 497, in _dispatch_request
    dispatcher.dispatch(req)
  File "/home/user/Workspace/t11294/teo-rjollos.git/trac/web/main.py", line 214, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/user/Workspace/t11294/teo-rjollos.git/trac/web/chrome.py", line 621, in process_request
    assert os.path.commonprefix([dir, path]) == dir
AssertionError: 

If I'm using the Image macro to display trac_logo.png from the shared directory, but that directory doesn't exist, I see:

03:05:29 PM Trac[chrome] WARNING: File trac_logo.png not found in any of [u'/var/www/shared/htdocs']

However, if [inherit] htdocs_dir = , or the option isn't defined in trac.ini, the traceback results.

If I change the code slightly to show the values being asserted:

03:09:21 PM Trac[main] ERROR: Internal Server Error: 
Traceback (most recent call last):
  File "/home/user/Workspace/t11294/teo-rjollos.git/trac/web/main.py", line 497, in _dispatch_request
    dispatcher.dispatch(req)
  File "/home/user/Workspace/t11294/teo-rjollos.git/trac/web/main.py", line 214, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/user/Workspace/t11294/teo-rjollos.git/trac/web/chrome.py", line 621, in process_request
    assert os.path.commonprefix([dir, path]) == dir, [os.path.commonprefix([dir, path]), dir]
AssertionError: ['', '.']

Another way to reproduce the traceback is to set [header_logo] src to a file in the shared htdocs_dir and leave [inherit] htdocs_dir empty.

If we remove the assertion, we get a more useful error in the log:

03:22:34 PM Trac[chrome] WARNING: File trac_logo.png not found in any of ['.']
03:22:34 PM Trac[main] WARNING: [127.0.0.1] HTTPNotFound: 404 Not Found (File trac_logo.png not found)

I'm not sure what the purpose of that assert statement is. It was added in [1888], and I can't come up with a case for which the assertion could have been false in [1188]. However, after [1892] the assertion can be false if os.normpath(dir) != dir.

Provided the assertion isn't needed, I propose to remove it and possibly improve the log message for the case that the user tries to serve an image file from the shared htdocs dir, but [inherit] htdocs_dir is empty or not defined.

[Jun Omae]

No. If without the assertion, a remote attacker can retrieve the system file using like this.

http://example.org/chrome/site/..%2Fconf%2Ftrac.ini
http://example.org/chrome/common/..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2Fetc%2Fpasswd

[Remy Blank]

If the assert has security implications, it should be changed to a test and a raise ASAP, because assertions are removed when running with -O, and we don't want people's Trac to become vulnerable just because they run with "optimizations" turned on.

[Ryan J Ollos]

I've prepared some changes based on Rémy's suggestion: rjollos.git:t11295.

I'm not sure if the TracErrors's message should be more descriptive than Invalid chrome path.