CVE-2023-40890 & CVE-2023-40889

[wqh17101]

there may be some vulns. https://nvd.nist.gov/vuln/detail/CVE-2023-40889 https://nvd.nist.gov/vuln/detail/CVE-2023-40890 Please confirm and fix it if necessary.

[pr-apes]

@mchehab,

many thanks for your work on this project and for releasing it under a free license.

Could you please take a look at these two vulnerabilities?

Since they are rated as critical (9.8 out of 10), they may hinder the deployment of zbar.

(I was about to recommend the use of zbarimg, but I'll have to wait until this is solved.)

Many thanks for your help and congratulations for your excellent work.

[sandrinr]

There is a fork addressing these two CVEs in a crude but hopefully effective manner: https://github.com/Raemi/zbar/commits/master

[pr-apes]

If this PR (d5857d3f7d0f5a243c517ee22762e1e3ddeb8db2) fixes those issues, it seems only to avoid the exploitation of the CVE.

That being said, it would be great to have a new release that contains this (if a proper fix cannot be offered).

[risicle]

The patch for CVE-2023-40890 in that branch is overly strict and causes zbar's tests to fail.

[bastien-roucaries]

@risicle Patch for CVE-2023-40890 is not acceptable for a library: it print to stderr and exit

[bastien-roucaries]

@risiclen Tested against POC

. From: Remi Meier <remi.meier@xorlab.com>
Date: Thu, 19 Oct 2023 22:35:13 +0000
Subject: Add bounds check for CVE-2023-40890

Add a check to avoid exploitation of the CVE.

[debian] do not hardcode size of array and return -1 in case of error, and return no symbol

bug-debian-security: https://security-tracker.debian.org/tracker/CVE-2023-40889
origin: https://github.com/mchehab/zbar/commit/d5857d3f7d0f5a243c517ee22762e1e3ddeb8db2
---
 zbar/decoder/databar.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Index: zbar-0.23.92/zbar/decoder/databar.c
===================================================================
--- zbar-0.23.92.orig/zbar/decoder/databar.c
+++ zbar-0.23.92/zbar/decoder/databar.c
@@ -23,6 +23,8 @@

 #include <config.h>
 #include <zbar.h>
+#include <stdlib.h>
+#include <stdio.h>

 #ifdef DEBUG_DATABAR
 # define DEBUG_LEVEL (DEBUG_DATABAR)
@@ -663,10 +665,11 @@ match_segment (zbar_decoder_t *dcode,
     return(ZBAR_DATABAR);
 }

-static inline unsigned
+static inline signed
 lookup_sequence (databar_segment_t *seg,
                  int fixed,
-                 int seq[22])
+                 int seq[22],
+                 const size_t maxsize)
 {
     unsigned n = seg->data / 211, i;
     const unsigned char *p;
@@ -676,6 +679,13 @@ lookup_sequence (databar_segment_t *seg,
     dbprintf(2, " {%d,%d:", i, n);
     p = exp_sequences + i;

+    if (n >= maxsize-1) {
+       // The loop below checks i<n and increments i by one within the loop
+       // when accessing seq[22]. For this to be safe, n needs to be < 21.
+       // See CVE-2023-40890.
+       return -1;
+    }
+
     fixed >>= 1;
     seq[0] = 0;
     seq[1] = 1;
@@ -755,10 +765,15 @@ match_segment_exp (zbar_decoder_t *dcode
             }

             if(!i) {
-                if(!lookup_sequence(seg, fixed, seq)) {
+                signed int lu = lookup_sequence(seg, fixed, seq, sizeof(seq)/sizeof(seq[0]));
+                if(!lu) {
                     dbprintf(2, "[nf]");
                     continue;
                 }
+                if(lu < 0) {
+                    dbprintf(1, " [aborted]\n");
+                    goto abort;
+                }
                 width = seg->width;
                 dbprintf(2, " A00@%d", j);
             }
@@ -829,6 +844,8 @@ match_segment_exp (zbar_decoder_t *dcode
     dcode->direction = (1 - 2 * (seg->side ^ seg->color)) * dir;
     dcode->modifiers = MOD(ZBAR_MOD_GS1);
     return(ZBAR_DATABAR_EXP);
+abort:
+    return (ZBAR_NONE);
 }
 #undef IDX

[bastien-roucaries]

@mchehab

[risicle]

@bastien-roucaries I think exiting is a "better than nothing" strategy when it's not obvious what facilities a library has for "soft" error handling, but your patch does look better.

[bastien-roucaries]

My patch was tested against POC. Return checksum error no overflow

[jubalh]

My patch was tested against POC.

Where can I find the POC? I see it neither on this issue nor on https://hackmd.io/@cspl/B1ZkFZv23.

@bastien-roucaries do I see it right that in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051724 you say that it is fixed for 0.23.90? In https://security-tracker.debian.org/tracker/CVE-2023-40890 it looks like its only fixed in buster under version 0.22-1 (via patch) though.

[bastien-roucaries]

I have asked privaltly to the reporter.

Yes it is fixed for debian unstable...0.23.90

[jubalh]

I have asked privaltly to the reporter.

I'll shoot you an email :)

Yes it is fixed for debian unstable...0.23.90

But https://security-tracker.debian.org/tracker/CVE-2023-40890 sais its fixed for buster and unstable is still vulnerable?

[carnil]

I have asked privaltly to the reporter.

I'll shoot you an email :)

Yes it is fixed for debian unstable...0.23.90

But https://security-tracker.debian.org/tracker/CVE-2023-40890 sais its fixed for buster and unstable is still vulnerable?

The Debian security team did not had the poc so we prefer to mark it for now with the note as attached above. Once it's confirmed fixed upstream and this is aligned with the done change in unstable we can update the metadata.

[jubalh]

The Debian security team did not had the poc so we prefer to mark it for now with the note as attached above. Once it's confirmed fixed upstream and this is aligned with the done change in unstable we can update the metadata.

Thank you for the clarification.

[jubalh]

The original reports:

• https://hackmd.io/@cspl/B1ZkFZv23
• https://hackmd.io/@cspl/H1PxPAUnn

have now been updated and include the POC.

[carnil]

@jubalh should they be attached here in this issue separately as well in case the reference disapears and have it associated with this upstream issue clearly?

[jubalh]

That might be a good idea. I considered attaching it as well but I didn't want to interfere with the reporters work. But I agree that it would be a better option!

[mdevaev]

This is a disaster. A lot of software and high-level libraries use zbar, and this CVE is still not closed.

@mchehab Could you pay attention to this issue?

[mchehab]

This is a disaster. A lot of software and high-level libraries use zbar, and this CVE is still not closed.

@mchehab Could you pay attention to this issue?

Sure. Was in vacations, returning back today. As there were already some fixes even applied to Debian, the best would be if you could open a PR with the fixes for me to review.

[mdevaev]

@mchehab Sorry, I'm not ready to take responsibility for proposing and bringing to acceptance other people's patches for CVE for a codebase with which I am not familiar. It would be great if you did it yourself, or someone who has been following this problem here for a long time. I'm here solely as a user who is prevented by these CVEs from implementing QR support in my project.

[jubalh]

@mchehab please see https://github.com/mchehab/zbar/pull/276

[jubalh]

I'll attach the POCs which I got from @wqh17101 and which are now available on their website.