Re: [libaacs-devel] [PATCH] Fix endless loop in _record

[Roland Fischer]

Hi!

You're completely right with your comments! Submitted a V2 of this patch 
yesterday that fixes the problems you've mentioned - I hope the revised 
version can be merged :)

br, Roland

On 10.12.2015 10:46, Petri Hintukainen wrote:
On ke, 2015-12-09 at 20:28 +0100, Roland Fischer wrote:
In case of a corrupt file it could happen that len get 0 in _record
and this results in an endless loop.
Created an exit condition for this case and fixed related
procedures too (they need to cope with the error-return-value
from _record).
Reason for change:https://github.com/OpenELEC/OpenELEC.tv/pull/4378
---
  src/libaacs/mkb.c | 28 ++++++++++++++++++++++++++++
  1 file changed, 28 insertions(+)

diff --git a/src/libaacs/mkb.c b/src/libaacs/mkb.c
index 275b269..9de4beb 100644
--- a/src/libaacs/mkb.c
+++ b/src/libaacs/mkb.c
@@ -52,6 +52,12 @@ static const uint8_t *_record(MKB *mkb, uint8_t
type, size_t *rec_len)
              return mkb->buf + pos;
          }
  
+        if (len == 0) {
+            BD_DEBUG(DBG_MKB, "Couldn't retrieved MKB record 0x%02x
- len=0 (%p)\n", type,
+                  (void*)(mkb->buf + pos));
+            break;
+        }
+
          pos += len;
      }
Good catch. This fixes also the case when record is missing from MKB
(MKB is padded with zeros).

@@ -108,6 +114,10 @@ uint8_t mkb_type(MKB *mkb)
  {
      const uint8_t *rec = _record(mkb, 0x10, NULL);
  
+    if (!rec) {
+        return 0;
+    }
+
      return MKINT_BE32(rec + 4);
  }
  
@@ -115,6 +125,9 @@ uint32_t mkb_version(MKB *mkb)
  {
      const uint8_t *rec = _record(mkb, 0x10, NULL);
  
+    if (!rec) {
+        return 0;
+    }
      return MKINT_BE32(rec + 8);
  }
  
@@ -130,6 +143,9 @@ const uint8_t *mkb_host_revokation_entries(MKB
*mkb, size_t *len)
  {
      const uint8_t *rec = _record(mkb, 0x21, len);
  
+    if (!rec) {
+        return NULL;
+    }
Useless, already checked at next line

     if (rec) {
          rec += 4;
          *len -= 4;
@@ -142,6 +158,9 @@ const uint8_t *mkb_drive_revokation_entries(MKB
*mkb, size_t *len)
  {
      const uint8_t *rec = _record(mkb, 0x20, len);
  
+    if (!rec) {
+        return NULL;
+    }
same

      if (rec) {
          rec += 4;
          *len -= 4;
@@ -153,6 +172,9 @@ const uint8_t *mkb_drive_revokation_entries(MKB
*mkb, size_t *len)
  const uint8_t *mkb_subdiff_records(MKB *mkb, size_t *len)
  {
      const uint8_t *rec = _record(mkb, 0x04, len) + 4;
+    if (!rec) {
+        return NULL;
+    }
      *len -= 4;
  
      return rec;
Won't work as expected (rec has already been incremented by 4).
See ex. mkb_drive_revokation_entries().

@@ -161,6 +183,9 @@ const uint8_t *mkb_subdiff_records(MKB *mkb,
size_t *len)
  const uint8_t *mkb_cvalues(MKB *mkb, size_t *len)
  {
      const uint8_t *rec = _record(mkb, 0x05, len) + 4;
+    if (!rec) {
+        return NULL;
+    }
      *len -= 4;
same
  
      return rec;
@@ -174,6 +199,9 @@ const uint8_t *mkb_mk_dv(MKB *mkb)
  const uint8_t *mkb_signature(MKB *mkb, size_t *len)
  {
      const uint8_t *rec = _record(mkb, 0x02, len);
+    if (!rec) {
+        return NULL;
+    }
      *len -= 4;
  
      return rec + 4;
Also mkb_mk_dv() needs check for NULL.


_______________________________________________
libaacs-devel mailing list
libaacs-devel@videolan.org
https://mailman.videolan.org/listinfo/libaacs-devel

_______________________________________________
libaacs-devel mailing list
libaacs-devel@videolan.org
https://mailman.videolan.org/listinfo/libaacs-devel