GNOME Bugzilla – Bug 788548
Function parameter assigned the address of a local auto-variable in function id3v2_frames_to_tag_list
Last modified: 2018-01-19 20:46:20 UTC
In File : id3v2.c Function : static gboolean id3v2_frames_to_tag_list (ID3TagsWorking * work, guint size) Line No. 574 In statement work->frame_id = frame_id; Function parameter work->frame_id of type gchar * (pointer) is assigned local auto-variable frame_id which is defined as Line No. 477: gchar frame_id[5] = ""; Function parameter is assigned the address of a local auto-variable. Local auto-variables are reserved from the stack which is freed when the function ends. The address is invalid after the function ends and it might 'leak' from the function through the parameter. Solution: frame_id should be initialized as below :- gchar *frame_id = g_malloc (5*sizeof(gchar)); Please let me know if I can submit a patch for the issue.
work->frame_id is only accessed by functions called within the block where frame_id is valid. So maybe it's not the nicest code structure, but it's not clear to me there's an actual bug here.
Created attachment 361018 [details] [review] [PATCH][Fix for] Function parameter assigned the address of a local auto-variable in function id3v2_frames_to_tag_list The attached patch resolves the possibility of a bug and a gives a better assignment to the function parameter. Please review and share the feedback
Created attachment 361019 [details] [review] [PATCH][Fix for] Function parameter assigned the address of a local auto-variable in function id3v2_frames_to_tag_list Please review this patch.
Created attachment 361020 [details] [review] [PATCH][Fix for] Function parameter assigned the address of a local auto-variable in function id3v2_frames_to_tag_list
Created attachment 361021 [details] [review] [PATCH][Fix for] Function parameter assigned the address of a local auto-variable in function id3v2_frames_to_tag_list
Created attachment 361024 [details] [review] [PATCH][Fix for] Function parameter assigned the address of a local auto-variable in function id3v2_frames_to_tag_list
I'm not in favour of this, sorry. It causes extra overhead without actually improving anything, it just makes your code checker happy. We still have an invalid (freed) pointer in the Works structure in the end. If we want to make sure that the local memory isn't accessed after it goes away, we should set the field in the Works structure to NULL instead.
Maybe this makes your checker happy: commit a1af74feda8f4e5aeae97da2aebd43d2fe8432eb (HEAD -> master) Author: Tim-Philipp Müller <tim@centricular.com> Date: Fri Jan 19 20:43:57 2018 +0000 tag: id3v2: don't leak stack pointer outside of block where it's valid https://bugzilla.gnome.org/show_bug.cgi?id=788548
Comment on attachment 361024 [details] [review] [PATCH][Fix for] Function parameter assigned the address of a local auto-variable in function id3v2_frames_to_tag_list As explained. Sorry :)