Bug 391146 - fix crash when some external APIs fail
Summary: fix crash when some external APIs fail
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Services/MP3tunes (show other bugs)
Version: 2.8.0
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: 2.9
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-27 07:13 UTC by Zhouyang
Modified: 2024-06-21 10:52 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
patch of reported bugs (1.40 KB, patch)
2018-02-27 07:13 UTC, Zhouyang
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Zhouyang 2018-02-27 07:13:42 UTC
Created attachment 111040 [details]
patch of reported bugs

Hi,

I'm a PhD student. I analyzed Amarok source code and found some potential API bugs that may cause crashes:

1. In src/services/mp3tunes/libmp3tunes/locker.c:1501:5, if "curl_easy_perform" failed, there would be unexcepted results.

2. In src/services/mp3tunes/libmp3tunes/locker.c:1479:14, if "fopen" failed, "hd_src" might cause a crash.

I think it's unsafe to assume the library function would be correct. It would be better if we could handle the error properly.

Attached please find the patch against version 2.8.0. Hopefully, it can solve these potential bugs.

Best,
Zhouyang
Comment 1 Myriam Schweingruber 2018-02-28 20:09:33 UTC
Thank you for the patch,but you should test this against current master which is several hundred commits ahead of what you have in version 2.8.0.

Also we are currently preparing a release of version 2.9.0, and the port to Qt5 is under way. If your patch still applies to current master, please submit it to http://phabricator.kde.org (you will need an account on http://identity.kde.org for this).
Comment 2 Zhouyang 2018-03-01 01:24:49 UTC
I've submitted the patch on http://phabricator.kde.org. Please check it.
Comment 3 Myriam Schweingruber 2018-09-26 08:04:18 UTC
Sorry, forgot to change status
Comment 4 Bug Janitor Service 2024-06-21 10:35:12 UTC
A possibly relevant merge request was started @ https://invent.kde.org/multimedia/amarok/-/merge_requests/106
Comment 5 Tuomas Nurmi 2024-06-21 10:52:36 UTC
Git commit 312c36046bf49dadb2edbb0398989ffb6230c3e0 by Tuomas Nurmi.
Committed on 21/06/2024 at 10:31.
Pushed by nurmi into branch 'master'.

Some theoretical mp3tunes fixes

Fix the regexes so it compiles again. Add fixes to reported bugs, including a patch
posted by Zhouyang on bugs.kde.org. Fix some issues reported by code quality scans.

The compilation works now, but no effort was made even to try to test the functionality,
which probably doesn't. The service as is would need work and everything is turned off
in CMakeLists. Hard to predict if someone will ever update the mp3tunes to usable state,
but I guess there's no point in NOT applying the potential fixes (as the service still
exists, so maybe not everything's not broken enough to justify removing) so the existing
issues can be happily cleaned up at least.
Related: bug 419132

M  +3    -2    src/services/mp3tunes/Mp3tunesService.cpp
M  +3    -3    src/services/mp3tunes/Mp3tunesServiceCollection.cpp
M  +27   -5    src/services/mp3tunes/libmp3tunes/locker.c

https://invent.kde.org/multimedia/amarok/-/commit/312c36046bf49dadb2edbb0398989ffb6230c3e0