dutzi / tamper

Devtools extension, lets you locally edit files served from the web (based on mitmproxy).
http://dutzi.github.io/tamper
MIT License
394 stars 36 forks source link

Local File Exposure #4

Closed mhils closed 10 years ago

mhils commented 10 years ago

Hi @dutzi,

just following the progress you're making here - looks great!

One thing I noticed when testing: https://github.com/dutzi/tamper/blob/edb9cc2c2842f521d4fb6bf9646ad4835b631ce7/mitmproxy-extension/chromeproxy/chromeproxy#L239 is prone to local file exposure, i.e.

curl --proxy http://127.0.0.1:8080 http://mitm.it/../../../../../../etc/passwd

You may want to drop that feature before a public release :wink:

Cheers, Max

dutzi commented 10 years ago

Hi, @mhils, thanks for the heads up and the kind words! :blush:

I'm actually having some issues with this feature. A solution for the issue you raised might be to allow access to only the four files actually needed, ignoring other requests.

The issue I am experiencing happens when I am trying to download the p12 cert file. I think it has something to do with this file being binary. Maybe something with how I'm using HTTPResponse. The thing is that when the browser requests that file, it only receives the first 137 bytes of it :confused:

I'm not that familiar with python, if you have the time, can you please have a look at it?

Thanks you very much!

mhils commented 10 years ago

See #5 :wink: I think your problem was that you were not reading the file in binary mode. In retrospective, the content-length header might be superfluous.

(Obvious disclaimer: sanitize your input! :) )

Cheers, Max

dutzi commented 10 years ago

Awesome! I merged #5 and sanitized the path. I checked it in without the content-length header since it did seem to add it later.

Thanks