Vulnerability: In all versions of libyang before 1.0-r5, a stack-based buffer overflow was discovered in the way libyang parses YANG files with a leaf of type "bits". An application that uses libyang to parse untrusted YANG files may be vulnerable to this flaw, which would allow an attacker to cause a denial of service or possibly gain code execution.
Vulnerability ID: CVE-2019-19333
CWE: CWE-787
Severity score: 6.4
Project: https://github.com/CESNET/libyang
Published: 2019-12-06
File: src/parser.c
Commit message: parser BUGFIX long identity name buffer overflow

STRING_OVERFLOW (CWE-120)
Commit author: Michal Vasko

Vulnerable version (before fix):
 lyp_precompile_pattern(struct ly_ctx *ctx, const char *pattern, pcre** pcre_cmp,  * @param[in] data2 If \p type is #LY_TYPE_BITS: (int *) type bit field length,  *                                #LY_TYPE_DEC64: (uint8_t *) number of fraction digits (position of the floating point),  *                                otherwise ignored.  * @return 1 if a conversion took place, 0 if the value was kept the same.  */ static int make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, void *data2) make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo     uint64_t unum;     uint8_t c;      switch (type) {     case LY_TYPE_BITS:         bits = (struct lys_type_bit **)data1; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo                 continue;             }             if (buf[0]) {                 sprintf(buf + strlen(buf), " %s", bits[i]->name);             } else {                 strcpy(buf, bits[i]->name);             }         } make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo      case LY_TYPE_INST:         exp = lyxp_parse_expr(ctx, *value);         LY_CHECK_ERR_RETURN(!exp, LOGINT(ctx), 0);          module_name = NULL;         count = 0; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo             /* copy WS */             if (i && ((end = exp->expr + exp->expr_pos[i - 1] + exp->tok_len[i - 1]) != cur_expr)) {                 if (count + (cur_expr - end) > buf_len) {                     LOGINT(ctx);                     lyxp_expr_free(exp);                     return 0;                 }                 strncpy(&buf[count], end, cur_expr - end);                 count += cur_expr - end; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo                 if (!module_name || strncmp(cur_expr, module_name, j)) {                     /* print module name with colon, it does not equal to the parent one */                     if (count + j > buf_len) {                         LOGINT(ctx);                         lyxp_expr_free(exp);                         return 0;                     }                     strncpy(&buf[count], cur_expr, j);                     count += j; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo                  /* copy the rest */                 if (count + (exp->tok_len[i] - j) > buf_len) {                     LOGINT(ctx);                     lyxp_expr_free(exp);                     return 0;                 }                 strncpy(&buf[count], end, exp->tok_len[i] - j);                 count += exp->tok_len[i] - j;             } else {                 if (count + exp->tok_len[i] > buf_len) {                     LOGINT(ctx);                     lyxp_expr_free(exp);                     return 0;                 }                 strncpy(&buf[count], &exp->expr[exp->expr_pos[i]], exp->tok_len[i]);                 count += exp->tok_len[i]; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo         if (count > buf_len) {             LOGINT(ctx);             lyxp_expr_free(exp);             return 0;         }         buf[count] = '\0';  make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo     }      return 0; }  static const char * lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             c = c + len;         }          make_canonical(ctx, LY_TYPE_BITS, value_, bits, &type->info.bits.count);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_DEC64, value_, &num, &type->info.dec64.dig);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             type->parent->flags |= LYS_DFLTJSON;         }          make_canonical(ctx, LY_TYPE_IDENT, &value, (void*)lys_main_module(local_mod)->name, NULL);          /* replace the old value with the new one (even if they may be the same) */         lydict_remove(ctx, *value_); lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             /* turn logging back on */             ly_ilo_restore(NULL, prev_ilo, NULL, 0);         } else {             if (make_canonical(ctx, LY_TYPE_INST, &value, NULL, NULL)) {                 /* if a change occurred, value was removed from the dictionary so fix the pointers */                 *value_ = value;             } lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_INT8, value_, &num, NULL);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_INT16, value_, &num, NULL);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_INT32, value_, &num, NULL);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_INT64, value_, &num, NULL);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_UINT8, value_, &unum, NULL);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_UINT16, value_, &unum, NULL);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_UINT32, value_, &unum, NULL);          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          make_canonical(ctx, LY_TYPE_UINT64, value_, &unum, NULL);          if (store) {             /* store the result */

Fixed version (after fix):
 lyp_precompile_pattern(struct ly_ctx *ctx, const char *pattern, pcre** pcre_cmp,  * @param[in] data2 If \p type is #LY_TYPE_BITS: (int *) type bit field length,  *                                #LY_TYPE_DEC64: (uint8_t *) number of fraction digits (position of the floating point),  *                                otherwise ignored.  * @return 1 if a conversion took place, 0 if the value was kept the same, -1 on error.  */ static int make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, void *data2) make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo     uint64_t unum;     uint8_t c;  #define LOGBUF(str) LOGERR(ctx, LY_EINVAL, "Value \"%s\" is too long.", str)      switch (type) {     case LY_TYPE_BITS:         bits = (struct lys_type_bit **)data1; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo                 continue;             }             if (buf[0]) {                 LY_CHECK_ERR_RETURN(strlen(buf) + 1 + strlen(bits[i]->name) > buf_len, LOGBUF(bits[i]->name), -1);                 sprintf(buf + strlen(buf), " %s", bits[i]->name);             } else {                 LY_CHECK_ERR_RETURN(strlen(bits[i]->name) > buf_len, LOGBUF(bits[i]->name), -1);                 strcpy(buf, bits[i]->name);             }         } make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo      case LY_TYPE_INST:         exp = lyxp_parse_expr(ctx, *value);         LY_CHECK_ERR_RETURN(!exp, LOGINT(ctx), -1);          module_name = NULL;         count = 0; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo             /* copy WS */             if (i && ((end = exp->expr + exp->expr_pos[i - 1] + exp->tok_len[i - 1]) != cur_expr)) {                 if (count + (cur_expr - end) > buf_len) {                     lyxp_expr_free(exp);                     LOGBUF(end);                     return -1;                 }                 strncpy(&buf[count], end, cur_expr - end);                 count += cur_expr - end; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo                 if (!module_name || strncmp(cur_expr, module_name, j)) {                     /* print module name with colon, it does not equal to the parent one */                     if (count + j > buf_len) {                         lyxp_expr_free(exp);                         LOGBUF(cur_expr);                         return -1;                     }                     strncpy(&buf[count], cur_expr, j);                     count += j; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo                  /* copy the rest */                 if (count + (exp->tok_len[i] - j) > buf_len) {                     lyxp_expr_free(exp);                     LOGBUF(end);                     return -1;                 }                 strncpy(&buf[count], end, exp->tok_len[i] - j);                 count += exp->tok_len[i] - j;             } else {                 if (count + exp->tok_len[i] > buf_len) {                     lyxp_expr_free(exp);                     LOGBUF(&exp->expr[exp->expr_pos[i]]);                     return -1;                 }                 strncpy(&buf[count], &exp->expr[exp->expr_pos[i]], exp->tok_len[i]);                 count += exp->tok_len[i]; make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo         if (count > buf_len) {             LOGINT(ctx);             lyxp_expr_free(exp);             return -1;         }         buf[count] = '\0';  make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo     }      return 0;  #undef LOGBUF }  static const char * lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             c = c + len;         }          if (make_canonical(ctx, LY_TYPE_BITS, value_, bits, &type->info.bits.count) == -1) {             free(bits);             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_DEC64, value_, &num, &type->info.dec64.dig) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             type->parent->flags |= LYS_DFLTJSON;         }          if (make_canonical(ctx, LY_TYPE_IDENT, &value, (void*)lys_main_module(local_mod)->name, NULL) == -1) {             lydict_remove(ctx, value);             goto error;         }          /* replace the old value with the new one (even if they may be the same) */         lydict_remove(ctx, *value_); lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             /* turn logging back on */             ly_ilo_restore(NULL, prev_ilo, NULL, 0);         } else {             if ((c = make_canonical(ctx, LY_TYPE_INST, &value, NULL, NULL))) {                 if (c == -1) {                     goto error;                 }                  /* if a change occurred, value was removed from the dictionary so fix the pointers */                 *value_ = value;             } lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_INT8, value_, &num, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_INT16, value_, &num, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_INT32, value_, &num, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_INT64, value_, &num, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_UINT8, value_, &unum, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_UINT16, value_, &unum, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_UINT32, value_, &unum, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x             goto error;         }          if (make_canonical(ctx, LY_TYPE_UINT64, value_, &unum, NULL) == -1) {             goto error;         }          if (store) {             /* store the result */

Patch:
patch: @@ -979,7 +979,7 @@ lyp_precompile_pattern(struct ly_ctx *ctx, const char *pattern, pcre** pcre_cmp,
  * @param[in] data2 If \p type is #LY_TYPE_BITS: (int *) type bit field length,
  *                                #LY_TYPE_DEC64: (uint8_t *) number of fraction digits (position of the floating point),
  *                                otherwise ignored.
- * @return 1 if a conversion took place, 0 if the value was kept the same.
+ * @return 1 if a conversion took place, 0 if the value was kept the same, -1 on error.
  */
 static int
 make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, void *data2)
@@ -994,6 +994,8 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
     uint64_t unum;
     uint8_t c;
 
+#define LOGBUF(str) LOGERR(ctx, LY_EINVAL, "Value \"%s\" is too long.", str)
+
     switch (type) {
     case LY_TYPE_BITS:
         bits = (struct lys_type_bit **)data1;
@@ -1006,8 +1008,10 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
                 continue;
             }
             if (buf[0]) {
+                LY_CHECK_ERR_RETURN(strlen(buf) + 1 + strlen(bits[i]->name) > buf_len, LOGBUF(bits[i]->name), -1);
                 sprintf(buf + strlen(buf), " %s", bits[i]->name);
             } else {
+                LY_CHECK_ERR_RETURN(strlen(bits[i]->name) > buf_len, LOGBUF(bits[i]->name), -1);
                 strcpy(buf, bits[i]->name);
             }
         }
@@ -1025,7 +1029,7 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
 
     case LY_TYPE_INST:
         exp = lyxp_parse_expr(ctx, *value);
-        LY_CHECK_ERR_RETURN(!exp, LOGINT(ctx), 0);
+        LY_CHECK_ERR_RETURN(!exp, LOGINT(ctx), -1);
 
         module_name = NULL;
         count = 0;
@@ -1035,9 +1039,9 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
             /* copy WS */
             if (i && ((end = exp->expr + exp->expr_pos[i - 1] + exp->tok_len[i - 1]) != cur_expr)) {
                 if (count + (cur_expr - end) > buf_len) {
-                    LOGINT(ctx);
                     lyxp_expr_free(exp);
-                    return 0;
+                    LOGBUF(end);
+                    return -1;
                 }
                 strncpy(&buf[count], end, cur_expr - end);
                 count += cur_expr - end;
@@ -1051,9 +1055,9 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
                 if (!module_name || strncmp(cur_expr, module_name, j)) {
                     /* print module name with colon, it does not equal to the parent one */
                     if (count + j > buf_len) {
-                        LOGINT(ctx);
                         lyxp_expr_free(exp);
-                        return 0;
+                        LOGBUF(cur_expr);
+                        return -1;
                     }
                     strncpy(&buf[count], cur_expr, j);
                     count += j;
@@ -1062,17 +1066,17 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
 
                 /* copy the rest */
                 if (count + (exp->tok_len[i] - j) > buf_len) {
-                    LOGINT(ctx);
                     lyxp_expr_free(exp);
-                    return 0;
+                    LOGBUF(end);
+                    return -1;
                 }
                 strncpy(&buf[count], end, exp->tok_len[i] - j);
                 count += exp->tok_len[i] - j;
             } else {
                 if (count + exp->tok_len[i] > buf_len) {
-                    LOGINT(ctx);
                     lyxp_expr_free(exp);
-                    return 0;
+                    LOGBUF(&exp->expr[exp->expr_pos[i]]);
+                    return -1;
                 }
                 strncpy(&buf[count], &exp->expr[exp->expr_pos[i]], exp->tok_len[i]);
                 count += exp->tok_len[i];
@@ -1081,7 +1085,7 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
         if (count > buf_len) {
             LOGINT(ctx);
             lyxp_expr_free(exp);
-            return 0;
+            return -1;
         }
         buf[count] = '\0';
 
@@ -1146,6 +1150,8 @@ make_canonical(struct ly_ctx *ctx, int type, const char **value, void *data1, vo
     }
 
     return 0;
+
+#undef LOGBUF
 }
 
 static const char *
@@ -1412,7 +1418,10 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             c = c + len;
         }
 
-        make_canonical(ctx, LY_TYPE_BITS, value_, bits, &type->info.bits.count);
+        if (make_canonical(ctx, LY_TYPE_BITS, value_, bits, &type->info.bits.count) == -1) {
+            free(bits);
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1470,7 +1479,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_DEC64, value_, &num, &type->info.dec64.dig);
+        if (make_canonical(ctx, LY_TYPE_DEC64, value_, &num, &type->info.dec64.dig) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1598,7 +1609,10 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             type->parent->flags |= LYS_DFLTJSON;
         }
 
-        make_canonical(ctx, LY_TYPE_IDENT, &value, (void*)lys_main_module(local_mod)->name, NULL);
+        if (make_canonical(ctx, LY_TYPE_IDENT, &value, (void*)lys_main_module(local_mod)->name, NULL) == -1) {
+            lydict_remove(ctx, value);
+            goto error;
+        }
 
         /* replace the old value with the new one (even if they may be the same) */
         lydict_remove(ctx, *value_);
@@ -1651,7 +1665,11 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             /* turn logging back on */
             ly_ilo_restore(NULL, prev_ilo, NULL, 0);
         } else {
-            if (make_canonical(ctx, LY_TYPE_INST, &value, NULL, NULL)) {
+            if ((c = make_canonical(ctx, LY_TYPE_INST, &value, NULL, NULL))) {
+                if (c == -1) {
+                    goto error;
+                }
+
                 /* if a change occurred, value was removed from the dictionary so fix the pointers */
                 *value_ = value;
             }
@@ -1749,7 +1767,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_INT8, value_, &num, NULL);
+        if (make_canonical(ctx, LY_TYPE_INT8, value_, &num, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1764,7 +1784,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_INT16, value_, &num, NULL);
+        if (make_canonical(ctx, LY_TYPE_INT16, value_, &num, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1779,7 +1801,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_INT32, value_, &num, NULL);
+        if (make_canonical(ctx, LY_TYPE_INT32, value_, &num, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1795,7 +1819,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_INT64, value_, &num, NULL);
+        if (make_canonical(ctx, LY_TYPE_INT64, value_, &num, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1810,7 +1836,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_UINT8, value_, &unum, NULL);
+        if (make_canonical(ctx, LY_TYPE_UINT8, value_, &unum, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1825,7 +1853,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_UINT16, value_, &unum, NULL);
+        if (make_canonical(ctx, LY_TYPE_UINT16, value_, &unum, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1840,7 +1870,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_UINT32, value_, &unum, NULL);
+        if (make_canonical(ctx, LY_TYPE_UINT32, value_, &unum, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
@@ -1855,7 +1887,9 @@ lyp_parse_value(struct lys_type *type, const char **value_, struct lyxml_elem *x
             goto error;
         }
 
-        make_canonical(ctx, LY_TYPE_UINT64, value_, &unum, NULL);
+        if (make_canonical(ctx, LY_TYPE_UINT64, value_, &unum, NULL) == -1) {
+            goto error;
+        }
 
         if (store) {
             /* store the result */
path: src/parser.c