xbmc: add fix for playing rar files (PR1147), this should fix #955

Signed-off-by: Stephan Raue <stephan@openelec.tv>
This commit is contained in:
Stephan Raue 2012-07-27 17:52:15 +02:00
parent 77737b6598
commit b99ed5d66d
2 changed files with 296 additions and 0 deletions

View File

@ -0,0 +1,148 @@
From f01f95af95f287847c850572abe16b6969967ba7 Mon Sep 17 00:00:00 2001
From: Anssi Hannula <anssi@xbmc.org>
Date: Wed, 16 May 2012 19:01:17 +0300
Subject: [PATCH 1/3] fixed: crashes with corrupted rar files
UnrarXLib does not handle invalid files gracefully enough, and some
files can cause it to tell CRarFile that it has written more data into
its buffer than actually fits there, causing CRarFile::Read() to
eventually overread the buffer.
Add checks in CRarFile for the validity of byte counts retrieved from
UnrarXLib to prevent crashes in such situations.
(cherry picked from commit ca9457286994ef0b021744797b8d78fb78260436)
---
xbmc/filesystem/FileRar.cpp | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/xbmc/filesystem/FileRar.cpp b/xbmc/filesystem/FileRar.cpp
index 9c87a35..d026f8f 100644
--- a/xbmc/filesystem/FileRar.cpp
+++ b/xbmc/filesystem/FileRar.cpp
@@ -310,6 +310,14 @@ unsigned int CFileRar::Read(void *lpBuf, int64_t uiBufSize)
m_iDataInBuffer = MAXWINMEMSIZE-m_pExtract->GetDataIO().UnpackToMemorySize;
+ if (m_iDataInBuffer < 0 ||
+ m_iDataInBuffer > MAXWINMEMSIZE - (m_szStartOfBuffer - m_szBuffer))
+ {
+ // invalid data returned by UnrarXLib, prevent a crash
+ CLog::Log(LOGERROR, "CRarFile::Read - Data buffer in inconsistent state");
+ m_iDataInBuffer = 0;
+ }
+
if (m_iDataInBuffer == 0)
break;
@@ -471,6 +479,15 @@ int64_t CFileRar::Seek(int64_t iFilePosition, int iWhence)
}
m_iDataInBuffer = m_pExtract->GetDataIO().m_iSeekTo; // keep data
m_iBufferStart = m_pExtract->GetDataIO().m_iStartOfBuffer;
+
+ if (m_iDataInBuffer < 0 || m_iDataInBuffer > MAXWINMEMSIZE)
+ {
+ // invalid data returned by UnrarXLib, prevent a crash
+ CLog::Log(LOGERROR, "CRarFile::Seek - Data buffer in inconsistent state");
+ m_iDataInBuffer = 0;
+ return -1;
+ }
+
m_szStartOfBuffer = m_szBuffer+MAXWINMEMSIZE-m_iDataInBuffer;
m_iFilePosition = iFilePosition;
--
1.7.10
From de1be4534cf410896b3102f95b6e02019ed64a90 Mon Sep 17 00:00:00 2001
From: Anssi Hannula <anssi@xbmc.org>
Date: Wed, 16 May 2012 17:13:07 +0300
Subject: [PATCH 2/3] fixed: rars that have unpacked size stored on first
volume only
Some multi-volume RAR files have their unpacked size set as 0 in all
volumes except the first one.
Use the previous unpacked size instead of 0 in such cases in order to
support such files properly.
(cherry picked from commit 683457d27736c09415a11d80933553f75139a253)
---
lib/UnrarXLib/volume.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/UnrarXLib/volume.cpp b/lib/UnrarXLib/volume.cpp
index 1f4d5e3..b24e98b 100644
--- a/lib/UnrarXLib/volume.cpp
+++ b/lib/UnrarXLib/volume.cpp
@@ -15,6 +15,7 @@ bool MergeArchive(Archive &Arc,ComprDataIO *DataIO,bool ShowFileName,char Comman
Log(Arc.FileName,St(MDataBadCRC),hd->FileName,Arc.FileName);
}
+ Int64 PrevFullUnpSize = hd->FullUnpSize;
Int64 PosBeforeClose=Arc.Tell();
Arc.Close();
@@ -144,6 +145,13 @@ bool MergeArchive(Archive &Arc,ComprDataIO *DataIO,bool ShowFileName,char Comman
}
}
#endif
+
+ if (hd->FullUnpSize == 0)
+ {
+ // some archives only have correct UnpSize in the first volume
+ hd->FullUnpSize = PrevFullUnpSize;
+ }
+
if (DataIO!=NULL)
{
if (HeaderType==ENDARC_HEAD)
--
1.7.10
From d7bed5ddbbc98d7fedac663410d8e7e64bdf20c7 Mon Sep 17 00:00:00 2001
From: Anssi Hannula <anssi@xbmc.org>
Date: Wed, 16 May 2012 00:14:49 +0300
Subject: [PATCH 3/3] fixed: CRarFile::Read() returning wrong data after some
seek patterns
Certain seek patterns on a file inside a non-compressed rar file can
cause CmdExtract::UnstoreFile() to think that the destination buffer has
been filled (as DestUnpSize counter, originally set to the file size,
reaches zero).
However, counting written bytes using DestUnpSize doesn't make sense for
the UnpackToMemory codepath used for non-compressed rar files, as there
can be seeks which can eventually cause more data to be read than what
the actual file size was. The actual output buffer is internally handled
by ComprDataIO.
The check in UnstoreFile() will result in not all data being written to
the destination buffer, causing CRarFile::Read() to return old stale
data.
Fix that by dropping the unnecessary DestUnpSize handling in
UnpackToMemory codepath of CmdExtract::UnstoreFile().
(cherry picked from commit 840cd4ce4ac8c781e7d35db2ed86d575a42c37e7)
---
lib/UnrarXLib/extract.cpp | 3 ---
1 file changed, 3 deletions(-)
diff --git a/lib/UnrarXLib/extract.cpp b/lib/UnrarXLib/extract.cpp
index b4a8091..368a899 100644
--- a/lib/UnrarXLib/extract.cpp
+++ b/lib/UnrarXLib/extract.cpp
@@ -863,10 +863,7 @@ void CmdExtract::UnstoreFile(ComprDataIO &DataIO,Int64 DestUnpSize)
}
if (Code > 0)
{
- Code=Code<DestUnpSize ? Code:int64to32(DestUnpSize);
DataIO.UnpWrite(&Buffer[0],Code);
- if (DestUnpSize>=0)
- DestUnpSize-=Code;
}
else
{
--
1.7.10

View File

@ -0,0 +1,148 @@
From f01f95af95f287847c850572abe16b6969967ba7 Mon Sep 17 00:00:00 2001
From: Anssi Hannula <anssi@xbmc.org>
Date: Wed, 16 May 2012 19:01:17 +0300
Subject: [PATCH 1/3] fixed: crashes with corrupted rar files
UnrarXLib does not handle invalid files gracefully enough, and some
files can cause it to tell CRarFile that it has written more data into
its buffer than actually fits there, causing CRarFile::Read() to
eventually overread the buffer.
Add checks in CRarFile for the validity of byte counts retrieved from
UnrarXLib to prevent crashes in such situations.
(cherry picked from commit ca9457286994ef0b021744797b8d78fb78260436)
---
xbmc/filesystem/FileRar.cpp | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/xbmc/filesystem/FileRar.cpp b/xbmc/filesystem/FileRar.cpp
index 9c87a35..d026f8f 100644
--- a/xbmc/filesystem/FileRar.cpp
+++ b/xbmc/filesystem/FileRar.cpp
@@ -310,6 +310,14 @@ unsigned int CFileRar::Read(void *lpBuf, int64_t uiBufSize)
m_iDataInBuffer = MAXWINMEMSIZE-m_pExtract->GetDataIO().UnpackToMemorySize;
+ if (m_iDataInBuffer < 0 ||
+ m_iDataInBuffer > MAXWINMEMSIZE - (m_szStartOfBuffer - m_szBuffer))
+ {
+ // invalid data returned by UnrarXLib, prevent a crash
+ CLog::Log(LOGERROR, "CRarFile::Read - Data buffer in inconsistent state");
+ m_iDataInBuffer = 0;
+ }
+
if (m_iDataInBuffer == 0)
break;
@@ -471,6 +479,15 @@ int64_t CFileRar::Seek(int64_t iFilePosition, int iWhence)
}
m_iDataInBuffer = m_pExtract->GetDataIO().m_iSeekTo; // keep data
m_iBufferStart = m_pExtract->GetDataIO().m_iStartOfBuffer;
+
+ if (m_iDataInBuffer < 0 || m_iDataInBuffer > MAXWINMEMSIZE)
+ {
+ // invalid data returned by UnrarXLib, prevent a crash
+ CLog::Log(LOGERROR, "CRarFile::Seek - Data buffer in inconsistent state");
+ m_iDataInBuffer = 0;
+ return -1;
+ }
+
m_szStartOfBuffer = m_szBuffer+MAXWINMEMSIZE-m_iDataInBuffer;
m_iFilePosition = iFilePosition;
--
1.7.10
From de1be4534cf410896b3102f95b6e02019ed64a90 Mon Sep 17 00:00:00 2001
From: Anssi Hannula <anssi@xbmc.org>
Date: Wed, 16 May 2012 17:13:07 +0300
Subject: [PATCH 2/3] fixed: rars that have unpacked size stored on first
volume only
Some multi-volume RAR files have their unpacked size set as 0 in all
volumes except the first one.
Use the previous unpacked size instead of 0 in such cases in order to
support such files properly.
(cherry picked from commit 683457d27736c09415a11d80933553f75139a253)
---
lib/UnrarXLib/volume.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/UnrarXLib/volume.cpp b/lib/UnrarXLib/volume.cpp
index 1f4d5e3..b24e98b 100644
--- a/lib/UnrarXLib/volume.cpp
+++ b/lib/UnrarXLib/volume.cpp
@@ -15,6 +15,7 @@ bool MergeArchive(Archive &Arc,ComprDataIO *DataIO,bool ShowFileName,char Comman
Log(Arc.FileName,St(MDataBadCRC),hd->FileName,Arc.FileName);
}
+ Int64 PrevFullUnpSize = hd->FullUnpSize;
Int64 PosBeforeClose=Arc.Tell();
Arc.Close();
@@ -144,6 +145,13 @@ bool MergeArchive(Archive &Arc,ComprDataIO *DataIO,bool ShowFileName,char Comman
}
}
#endif
+
+ if (hd->FullUnpSize == 0)
+ {
+ // some archives only have correct UnpSize in the first volume
+ hd->FullUnpSize = PrevFullUnpSize;
+ }
+
if (DataIO!=NULL)
{
if (HeaderType==ENDARC_HEAD)
--
1.7.10
From d7bed5ddbbc98d7fedac663410d8e7e64bdf20c7 Mon Sep 17 00:00:00 2001
From: Anssi Hannula <anssi@xbmc.org>
Date: Wed, 16 May 2012 00:14:49 +0300
Subject: [PATCH 3/3] fixed: CRarFile::Read() returning wrong data after some
seek patterns
Certain seek patterns on a file inside a non-compressed rar file can
cause CmdExtract::UnstoreFile() to think that the destination buffer has
been filled (as DestUnpSize counter, originally set to the file size,
reaches zero).
However, counting written bytes using DestUnpSize doesn't make sense for
the UnpackToMemory codepath used for non-compressed rar files, as there
can be seeks which can eventually cause more data to be read than what
the actual file size was. The actual output buffer is internally handled
by ComprDataIO.
The check in UnstoreFile() will result in not all data being written to
the destination buffer, causing CRarFile::Read() to return old stale
data.
Fix that by dropping the unnecessary DestUnpSize handling in
UnpackToMemory codepath of CmdExtract::UnstoreFile().
(cherry picked from commit 840cd4ce4ac8c781e7d35db2ed86d575a42c37e7)
---
lib/UnrarXLib/extract.cpp | 3 ---
1 file changed, 3 deletions(-)
diff --git a/lib/UnrarXLib/extract.cpp b/lib/UnrarXLib/extract.cpp
index b4a8091..368a899 100644
--- a/lib/UnrarXLib/extract.cpp
+++ b/lib/UnrarXLib/extract.cpp
@@ -863,10 +863,7 @@ void CmdExtract::UnstoreFile(ComprDataIO &DataIO,Int64 DestUnpSize)
}
if (Code > 0)
{
- Code=Code<DestUnpSize ? Code:int64to32(DestUnpSize);
DataIO.UnpWrite(&Buffer[0],Code);
- if (DestUnpSize>=0)
- DestUnpSize-=Code;
}
else
{
--
1.7.10