Chapters out-of-bounds bugfix

Archive of historical development discussions
Discussions / Development has moved to GitHub
Forum rules
*******************************
Please be aware we are now using GitHub for issue tracking and feature requests.
- This section of the forum is now closed to new topics.

*******************************
Post Reply
alxsrg
Posts: 3
Joined: Thu Aug 08, 2013 11:30 am

Chapters out-of-bounds bugfix

Post by alxsrg »

Hi all,

I've tried to use HandBrake and I've found it perfect except one thing.

I'm not a DVD specialist and I was confused by "titles" and "chapters".
I've tried to make a DVD backup copy and specified chapter 6 instead of title 6. There was no chapter 6 on my DVD and HandBrake crashed with a "Segmentation fault" error.

Well, I've done a little investigation and I've found at least 2 problems.
The first one is in hb_sync_init function; the problem is in the following snippet of code:

Code: Select all

for( i = job->chapter_start; i <= job->chapter_end; i++ )
{
chapter   = hb_list_item( job->list_chapter, i - 1 );
duration += chapter->duration;
}
chapter may be NULL here.

the second problem is related with ReadLoop() in reader.c

Code: Select all

hb_chapter_t *chap = hb_list_item( r->job->list_chapter, chapter_end - 1 );
There is no bounds check here also and chap may be NULL if user specifies invalid (out of range) chapter.

I'm not sure if there are other places with similar problem.

I'm not familiar with program architecture and I'm not sure where the bounds check should be placed to fix the problem once and forever.
I do not have much time but I will try to solve this issue.

If someone of developers picks this task - please, just inform me cause I just don't want to do the job that has been already done :D
alxsrg
Posts: 3
Joined: Thu Aug 08, 2013 11:30 am

Re: Chapters out-of-bounds bugfix

Post by alxsrg »

I can't register at https://reviews.handbrake.fr/account/register/ :(
The patch is placed below.

I will appreciate if someone helps me with a registration or just submit the patch into Reviews by himself.

Code: Select all

Index: libhb/reader.c
===================================================================
--- libhb/reader.c	(revision 5693)
+++ libhb/reader.c	(working copy)
@@ -398,7 +398,13 @@
          * a media chapter that got merged, we'll stop ripping too early.
          */
         int start = r->job->chapter_start;
-        hb_chapter_t *chap = hb_list_item( r->job->list_chapter, chapter_end - 1 );
+        const int chapterIdx = chapter_end - 1;
+        hb_chapter_t *chap = hb_list_item( r->job->list_chapter, chapterIdx );
+        if (NULL == chap) {
+            hb_error("No such chapter: %i\n", chapterIdx + 1);
+            *r->die = 1;
+            return;
+        }
 
         chapter_end = chap->index;
         if (start > 1)
Index: libhb/sync.c
===================================================================
--- libhb/sync.c	(revision 5693)
+++ libhb/sync.c	(working copy)
@@ -178,8 +178,14 @@
             duration = 0;
             for( i = job->chapter_start; i <= job->chapter_end; i++ )
             {
-                chapter   = hb_list_item( job->list_chapter, i - 1 );
-                duration += chapter->duration;
+                const int chapterIdx = i - 1;
+                chapter   = hb_list_item( job->list_chapter, chapterIdx );
+                if (NULL != chapter) {
+                    duration += chapter->duration;
+                } else {
+                    hb_error("No such chapter: %i\n", chapterIdx + 1);
+                    return NULL;
+                }
             }
         }
         sync->count_frames_max = duration * title->rate / title->rate_base / 90000;
Index: libhb/work.c
===================================================================
--- libhb/work.c	(revision 5693)
+++ libhb/work.c	(working copy)
@@ -947,6 +947,10 @@
 
     /* Synchronization */
     sync = hb_sync_init( job );
+    if (NULL == sync) {
+                    *job->die = 1;
+                    goto cleanup;
+    }
 
     /* Video decoder */
     int vcodec = title->video_codec? title->video_codec : WORK_DECMPEG2;
User avatar
s55
HandBrake Team
Posts: 10350
Joined: Sun Dec 24, 2006 1:05 pm

Re: Chapters out-of-bounds bugfix

Post by s55 »

Thanks

Try the registration again. ReCapatcha was broken :(. Hopefully fixed now.
BlueCrypt

Re: Chapters out-of-bounds bugfix

Post by BlueCrypt »

s55 wrote:Try the registration again. ReCapatcha was broken :(. Hopefully fixed now.
Not working.
User avatar
s55
HandBrake Team
Posts: 10350
Joined: Sun Dec 24, 2006 1:05 pm

Re: Chapters out-of-bounds bugfix

Post by s55 »

Try again now. Capatcha disabled.
alxsrg
Posts: 3
Joined: Thu Aug 08, 2013 11:30 am

Re: Chapters out-of-bounds bugfix

Post by alxsrg »

Post Reply