search

RSS-Bridge / rss-bridge

The RSS feed for websites missing it
https://rss-bridge.org/bridge01/
The Unlicense
6.99k stars 1.02k forks source link

fix(FDroidRepoBridge): unlink when json file is absent from archive #4056

Closed dvikan closed 2 months ago

dvikan commented 2 months ago

mainly fix issue as described in subject.

this is one of two locations where a bridge writes directly to os via file_put_contents. I reviewed it and think it's fine securitywise.

@Mynacol

github-actions[bot] commented 2 months ago

Pull request artifacts

Bridge Context Status
FDroidRepo 1 Latest Updates (current) ❌ Bridge returned error 404! (19817)
❌ Type: HttpException
❌ Message: https://srv.tt-rss.org/fdroid/repo/index-v1.jar resulted in 404 Not Found <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
⚠️ WARNING Trying to access array offset on value of type null at bridges/FDroidRepoBridge.php line 81
FDroidRepo 1 Latest Updates (pr) ❌ Bridge returned error 404! (19817)
❌ Type: HttpException
❌ Message: https://srv.tt-rss.org/fdroid/repo/index-v1.jar resulted in 404 Not Found <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
⚠️ WARNING Trying to access array offset on value of type null at bridges/FDroidRepoBridge.php line 208
FDroidRepo 2 Follow Package (current) ❌ Bridge returned error 404! (19817)
❌ Type: HttpException
❌ Message: https://srv.tt-rss.org/fdroid/repo/index-v1.jar resulted in 404 Not Found <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
⚠️ WARNING Trying to access array offset on value of type null at bridges/FDroidRepoBridge.php line 81
FDroidRepo 2 Follow Package (pr) ❌ Bridge returned error 404! (19817)
❌ Type: HttpException
❌ Message: https://srv.tt-rss.org/fdroid/repo/index-v1.jar resulted in 404 Not Found <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
⚠️ WARNING Trying to access array offset on value of type null at bridges/FDroidRepoBridge.php line 208

last change: Thursday 2024-04-04 15:40:02

Mynacol commented 2 months ago

I think security is fine. We control the whole file name. The only minor concern would be a file confusion where the original file was replaced with another one before calling unlink on it, deleting the wrong file. Highly unlikely, as we use temporary file names, but still possible.

I looked into avoiding the writing-to-file and then reading-from-file altogether. The results are in the referenced issue below.