From patchwork Wed May 22 16:10:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier Maignial X-Patchwork-Id: 13252 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 7D872447AED for ; Wed, 22 May 2019 19:18:43 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5E6AE689FCE; Wed, 22 May 2019 19:18:43 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C764E68019C for ; Wed, 22 May 2019 19:18:36 +0300 (EEST) Received: by mail-wr1-f67.google.com with SMTP id f8so3002201wrt.1 for ; Wed, 22 May 2019 09:18:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=smile-fr.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=mYJCC/CxXevES+QFHBwVeHTzEiPhZNkuKUiSXlcCBkU=; b=O9gdZrbOLGUR/EH34cVuJXljmaiC8rI7VWi08Gi5TgclDFi/x/5dszYsiq+QrflpzQ oQ/95APCXs/aQyhdaf30gHN2jg9xiWLN+YnMD72fYVQSktO7pfzjZCORrCp9iAqS1KeK UQ4A1kD9+vS9zHNcMMdDKv+MzLbbzghFPzqDpG5AASbuCqTv3mntEh8zzEnfL5qUOwVq sg0iLC2aPxoRUyb5zZ0YupcqTeRgNe+zh5sLb+r124duZ7GuXPEXCDD3l8Lex6XWz1tw 2IquMTCOR8MTkwwsQgxBETHPuycdIKLm5pwiADU2WNfbH5zsu1c1UbpNvuPNeJKBxiMR SqPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=mYJCC/CxXevES+QFHBwVeHTzEiPhZNkuKUiSXlcCBkU=; b=bVEewLKh7VNOJY4sPKn/CyK5X7Dm5tu67nnEOp6QDuJN/JIxARjqhyFEP6SbMBS5DI fS6JeoYB9Iu5HxFWNGZtjxS9vLNahL9lwR39dEM5lRpXNvdFIeC/fBhndU10uyNYPUjU mYLRpN+j5QKK2X9UI8h+/HxNzytnO8ZEaXjKoXuALcqZk4V09Qr8sop6GA2SqK4TEx8H be3IMcCCtRnZlyG2BFiUMoX9bpXtjsghMOBofbQSNTeHO6Is0JtxJumrcDNDj2QAUEUC zZn5TSGI/j9Cdt6017jQOhD0EVI5NbUrSpFltRD75dOvFR0SbgU5LLLk7eGBesXnAOZZ 7RAA== X-Gm-Message-State: APjAAAXqjszyniqVKuegzH/XGbnrlEjWhGbglu7Fs4k3zzG2wRfxyNrm iHMQ65jFTWdPsb5M7v/qfm/c0dfiR9k= X-Google-Smtp-Source: APXvYqz80QrxvYhXXHqjrOTtCe/k5fCNFZs/FYKA95XBJeDb8L/v70/m8g8+vjKCPjz9WY7UG0XDbA== X-Received: by 2002:a5d:4fd2:: with SMTP id h18mr21150275wrw.117.1558541441097; Wed, 22 May 2019 09:10:41 -0700 (PDT) Received: from P-TLS-SASUKE-OLMAI.tagtec.fr (myfox-157-50.fib.nerim.net. [194.79.157.50]) by smtp.gmail.com with ESMTPSA id g13sm27653552wrw.63.2019.05.22.09.10.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 22 May 2019 09:10:40 -0700 (PDT) From: Olivier Maignial To: ffmpeg-devel@ffmpeg.org Date: Wed, 22 May 2019 18:10:36 +0200 Message-Id: <1558541436-2054-1-git-send-email-olivier.maignial@smile.fr> X-Mailer: git-send-email 2.7.4 Subject: [FFmpeg-devel] [PATCH v5] Fix integer parameters size check in SDP fmtp line X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: lu_zero@gentoo.org, Olivier Maignial MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" === PROBLEM === I was trying to record h264 + aac streams from an RTSP server to mp4 file. using this command line: ffmpeg -v verbose -y -i "rtsp:///my_resources" -codec copy -bsf:a aac_adtstoasc test.mp4 FFmpeg then fail to record audio and output this logs: [rtsp @ 0xcda1f0] The profile-level-id field size is invalid (40) [rtsp @ 0xcda1f0] Error parsing AU headers ... [rtsp @ 0xcda1f0] Could not find codec parameters for stream 1 (Audio: aac, 48000 Hz, 1 channels): unspecified sample format In SDP provided by my RTSP server I had this fmtp line: a=fmtp:98 streamType=5; profile-level-id=40; mode=AAC-hbr; config=1188; sizeLength=13; indexLength=3; indexDeltaLength=3; In FFmpeg code, I found a check introduced by commit 24130234cd9dd733116d17b724ea4c8e12ce097a. It disallows values greater than 32 for fmtp line parameters. However, In RFC-6416 (RTP Payload Format for MPEG-4 Audio/Visual Streams) give examples of "profile-level-id" values for AAC, up to 55. Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any limit of size on interger parameters given in fmtp line. === FIX === Instead of prohibit values over 32, I propose to check the possible integer overflow. The use of strtol allow to check the string validity and the possible overflow. To store and check return of strtol I use "long int" type and LONG_MIN/MAX definitions despite differences on 32/64bits platforms. It is consistent with the strtol man page, and it is the only way to check if overflow or underflow is detected by strtol. As the value is later checked against INT32_MIN and INT32_MAX, the behavior will finnaly be the same on both type of platform. This patch fix my problem and I now can record my RTSP AAC stream to mp4. It has passed the full fate tests suite sucessfully. Signed-off-by: Olivier Maignial --- Changes V4 -> V5: - Check value against INT32_MAX/MIN instead of INT_MAX/MIN which can be defferent depending on platform libavformat/rtpdec_mpeg4.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c index 4f70599..cf35afb 100644 --- a/libavformat/rtpdec_mpeg4.c +++ b/libavformat/rtpdec_mpeg4.c @@ -289,15 +289,33 @@ static int parse_fmtp(AVFormatContext *s, for (i = 0; attr_names[i].str; ++i) { if (!av_strcasecmp(attr, attr_names[i].str)) { if (attr_names[i].type == ATTR_NAME_TYPE_INT) { - int val = atoi(value); - if (val > 32) { + char *end_ptr = NULL; + errno = 0; + long int val = strtol(value, &end_ptr, 10); + if (value[0] == '\n' || end_ptr[0] != '\0') { av_log(s, AV_LOG_ERROR, - "The %s field size is invalid (%d)\n", - attr, val); + "The %s field value is not a number (%s)\n", + attr, value); return AVERROR_INVALIDDATA; } + if ((val == LONG_MAX && errno == ERANGE) || + val > INT32_MAX) { + av_log(s, AV_LOG_ERROR, + "Value of field %s overflows maximum integer value.\n", + attr); + return AVERROR_INVALIDDATA; + } + if ((val == LONG_MIN && errno == ERANGE) || + val < INT32_MIN) + { + av_log(s, AV_LOG_ERROR, + "Value of field %s underflows minimum integer value.\n", + attr); + return AVERROR_INVALIDDATA; + } + *(int *)((char *)data+ - attr_names[i].offset) = val; + attr_names[i].offset) = (int) val; } else if (attr_names[i].type == ATTR_NAME_TYPE_STR) { char *val = av_strdup(value); if (!val)