Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unescaped js string #1414

Closed
privatissimo opened this issue Oct 16, 2021 · 14 comments
Closed

Unescaped js string #1414

privatissimo opened this issue Oct 16, 2021 · 14 comments
Assignees
Labels
BUG 🐞 Not working as expected

Comments

@privatissimo
Copy link

Hi, there is an issue in OpenWebif with the link to stream the current service (tooltip "Stream:") located in the top right on the OpenWebif page ("osd" div).
When a recording is played and the filename (i.e. the name of the recorded program/transmission) contains one or more single quotes (that is an apostrophe, so is relatively common in some language) the browser (chrome in my test) console show the error "Uncaught SyntaxError: missing ) after argument list".

The problem is that internally the onclick event call the jumper80() javascript function (remember, when playing a recorded program) with the filename as argument and the filename is not properly escaped, so if it contains a single quote (and potentially other characters?).......it breaks.

For the record I'm using OWIF 1.4.8 with VU+ Suo4K SE with OpenPli 8.1-release build 2021-09-15, last update 2021-10-12, but I think the problem exists in any decoder using OpenWebif.

@privatissimo privatissimo added the BUG 🐞 Not working as expected label Oct 16, 2021
@jbleyel
Copy link
Contributor

jbleyel commented Oct 16, 2021

Do you use classic or modern interface?
Please provide the full filename including the full path if the path has extra signs.

@privatissimo
Copy link
Author

privatissimo commented Oct 16, 2021

I'm using classic interface, I admit I didn't even know there was a modern i/f !
This is the response (logged using chrome inspect->network) from the "polling" of the browser to the url http://WebIfAddress/api/statusinfo?_= while the recording is played:

{
"inStandby": "false",
"currservice_begin": "21:15",
"isStreaming": "false",
"currservice_begin_timestamp": 1634152500,
"currservice_end_timestamp": 1634166000,
"muted": false,
"isRecording": "false",
"currservice_station": "Non e' l'Arena",
"currservice_serviceref": "1:0:0:0:0:0:0:0:0:0:/media/hdd/movie/20211013 2110 - LA7 HD - Non e' l'Arena.ts",
"currservice_description": "Intrattenimento - Show - Ep. 3",
"volume": 100,
"currservice_fulldescription": "Non e' l'Arena\n21:15 - 01:00\n\nIl programma di Massimo Giletti che punta i riflettori sui temi e i protagonisti della piu' stretta attualita' politica, sociale e della cronaca.",
"currservice_name": "Non e' l'Arena",
"currservice_filename": "/media/hdd/movie/20211013 2110 - LA7 HD - Non e' l'Arena.ts",
"transcoding": true,
"currservice_end": "01:00",
"currservice_id": 12340,
"Streaming_list": ""
}

I believe/hope this provide all the necessary info.
I think the property used in the link is currservice_filename

@privatissimo
Copy link
Author

Just tested with modern interface, it still fail but differently, using modern the new page/tab opened by the link point to:
http://WebIfAddress/web/ts.m3u?file=/media/hdd/movie/20211013%202110%20-%20LA7%20HD%20-%20Non%20e

So, the url is truncated at the first single quote.

@WanWizard
Copy link
Contributor

WanWizard commented Oct 16, 2021

Imho you can't make any assumption on what is and what is not in a filename.

In the case of a recording, you can have something like:

-rwxrwxrwx    1 openpli  users          732 Oct 13 23:44 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY.eit
-rwxrwxrwx    1 openpli  users    210223104 Oct 13 23:51 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY.ts
-rwxrwxrwx    1 openpli  users          269 Oct 13 23:52 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY.ts.meta
-rwxrwxrwx    1 openpli  users       147984 Oct 13 23:51 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY.ts.sc
-rwxrwxrwx    1 openpli  users    3099443764 Oct 14 01:00 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY_001.ts
-rwxrwxrwx    1 openpli  users        69232 Oct 14 01:00 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY_001.ts.ap
-rwxrwxrwx    1 openpli  users           12 Oct 14 01:00 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY_001.ts.cuts
-rwxrwxrwx    1 openpli  users          271 Oct 14 22:30 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY_001.ts.meta
-rwxrwxrwx    1 openpli  users      1645808 Oct 14 00:59 20211013_2345_-_BBC_FOUR_HD_-_ORBIT__EARTH'S_EXTRAORDINARY_JOURNEY_001.ts.sc

but it could also be something that is downloaded. The name should be properly URL encoded if it is used in a URL, and that includes all special chareacters, not only spaces. So the name needs to be urllib quoted...

@WanWizard
Copy link
Contributor

Imho there is a difference between "characters not allowed in a filename" (which is dicated by the filesystem), and "characters not allowed in a URI", which is dictacted by the relevant RFC's. And you shouldn't mix the two.

If the URL in the m3u is properly quoted, the characters used in the filename are no longer relevant.

Don't fix an issue on the left by tweaking code on the right (and it also doesn't solve the issue with downloaded files).

@wedebe
Copy link
Collaborator

wedebe commented Oct 16, 2021

I agree with @WanWizard 's statement that you can't make any assumption on what is and what is not in a filename - EPGCache for example still lets through hazardous characters. We should always assume the worst and sanitise by default

@jbleyel
Copy link
Contributor

jbleyel commented Oct 16, 2021

I cannot reproduce this on my box.
My filename is 123'".ts

The result html of the stream button is:

<a href="#" onclick="jumper80('/media/hdd/movie/TEST/123%27%22.ts');" title="Stream: 123''"><div><i class="fa fa-desktop"></i></div></a>

@jbleyel
Copy link
Contributor

jbleyel commented Oct 16, 2021

Can you please check the filename encoding?
Is this really utf-8?

@privatissimo
Copy link
Author

Yes, the filename encoding is utf8, but since all characters are low ascii (<127) I think it makes little difference.

I find it strange that in your box the filename is escaped, in your box is it escaped by the backend service (info.py) or by the js in the browser (openwebif.js)? How does the json from /api/statusifo looks like? (see my second post here)

Looking at both (info.py and openwebif.js) in the master branch here in github, I cannot find where the filename is escaped, are you running OpenWebif code from the master branch?

@jbleyel
Copy link
Contributor

jbleyel commented Oct 17, 2021

Yes .. i'm using the latest version from the master here.
But i'm running ATV 7.0 and this is python3.

@privatissimo
Copy link
Author

I just installed ATV 7.0 (openatv-7.0-vuduo4kse-20211017_usb) and it fail ("Uncaught SyntaxError: missing ) after argument list" in chrome console) exactly the same as using PLI image.

In the browser the relevant (osdicon div) html part is:

<div id="osdicon"><a href="#" onclick="jumper80('/media/hdd/movie/20211013 2110 - LA7 HD - Non e' l'Arena.ts:Non e' l'Arena')" ;="" title="Stream: Non e" l'arena'=""><i class="fa fa-desktop"></i></a><a href="#" onclick="jumper8003('/media/hdd/movie/20211013 2110 - LA7 HD - Non e' l'Arena.ts:Non e' l'Arena')" ;="" title="Stream (transcoded): Non e" l'arena'=""><i class="fa fa-mobile"></i></a></div><span class="osdch">Non e' l'Arena</span>&nbsp;&nbsp;21:15 - 01:00&nbsp;&nbsp;Non e' l'Arena</div>

From your box, is currservice_filename encoded in the json from /api/statusifo?

@jbleyel
Copy link
Contributor

jbleyel commented Oct 17, 2021

Please provide all files as zip from the recording except the big .TS file.
You should create the zip or gz archive directly on the box.

@privatissimo
Copy link
Author

privatissimo commented Oct 17, 2021

test-archive.tar.gz
I included to the gz all files except .ts and .sc.

How did you (re)named your 123'".ts test file?

@privatissimo
Copy link
Author

The stream (download) "button" is fixed and works, the title (tooltip) is still unescaped so it's not displayed correctly (it is truncated) but....that's a minor issue

@wedebe wedebe self-assigned this Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG 🐞 Not working as expected
Projects
None yet
Development

No branches or pull requests

4 participants