From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 521954BA45 for ; Sat, 31 May 2025 16:21:50 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 716AB68DACC; Sat, 31 May 2025 19:21:45 +0300 (EEST) Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id F0CFC68D4C1 for ; Sat, 31 May 2025 19:21:38 +0300 (EEST) Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-451d3f72391so2839205e9.3 for ; Sat, 31 May 2025 09:21:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20230601.gappssmtp.com; s=20230601; t=1748708498; x=1749313298; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=0/tVMmmCYjpx1Nt+lVwpMrqTiII9fQxiH0Hj4Kf4dWU=; b=NsV+1RIqU3jQJZWeh2JuMFyKNcZeO4p75WyapJsq7xbyP39VTbDeCl61RLFeve383c Zx61+qhjwShkKToTkcykNn1OAoHiBiOdO1VjshRBgh/sy/UTICCYpi3gSVz9TUjyjaeG Bk27l2jXPhhm6Wcr+hEUbN3TZjgOowVy/wzFKiVA1AVcz09VyGJ75Y7XWb9A+P2P6TQ0 lQ7Bl/lL2uo2HP7TQVCbgeDoFjwgIs8n+VvJxCIgZObIGwdQhQZKogKLqdZ7wnMu30cb g3cEp+SilJfNXi/jkT+bm7jlBV8PFqiLUGjARaeQPu0OTdEp6PG5yRJEtulKr0Pgxpi7 XlxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1748708498; x=1749313298; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=0/tVMmmCYjpx1Nt+lVwpMrqTiII9fQxiH0Hj4Kf4dWU=; b=F6NJ87IDaRflgfTWl4mM+kHNb4s9Bruyi4oCqJnQjkYTLHACJuHXTmSymYw1lq3r3W rHBNj56R7dsQU0YqmtsPY2UnZm2T/BIgFYbqLwnKujmSRhtwX8JHQLYI802dAIAhzjPk Y43f+DchHzNwqwWPR4+BOa0sbnaiC/cz9dvOY9IEi1StgzOuKbCWUtEUcbenyHyOFlC/ e8fwnyHzv2pqnRR2CjQtmd7FATakVxtd4m10L0/zDdNO/iGLzXWcxtFgw8xFLiRPcItH nH+VGSWHQT8q/COV/IV9r2fiBCHnlbi0olBg6vx2mhSGVvsJwy9CYpeWNFhzbybj4M4i k5uQ== X-Gm-Message-State: AOJu0YxiIKFMEpFc3z07VVPK6Tnb4HcOGgSimpYR6gewh++HnIOqbO0H Lq/hteeIqth76Aux+/rFraANtVc/hlV9qAilMqcKKJFNaXTlO4BiRPa5z3KnVZEYsLUry+SgT1L XsYCY X-Gm-Gg: ASbGnctTCFgzKyPWPk8GNH5ov+Fm7X4wToDt6mhvVlFs7+9UJfbWkBdyGiPIvxyYD88 gno9RbnONXjNHLSvyaBvmMD1ZdTu4ku69rjq7rAd1G2AZ1yWDaQjYdGoDssFZ1ZlEe481axvDfT kphVYEFR8fyr6Ouwo547v1EPwAPA4ADKVE+X1b5mh++HxAa4ywnsgq6WVnVR4jQfWtyrN0Lb9a8 HygKxEI5r3nxeQo/eQ2PqA8dTANZ7k3hdlCfBgRHMEBdPIQzoOaDD/1TXln8SNS/ZwZgKKTpqre HmKPo5ndGtZYBUa3iAfJxzO9VPdK0J6ZReDYFFdHYPK6X9kwZqbr2fcrJcesBPlgj4fPQ6d5g+D gp5nvsxV9QeRsjWsOCbTJ7Mof X-Google-Smtp-Source: AGHT+IEhISZxyhgZN59NeVHb0A7YAGrtpv1MRWJlrzZClmCB03OmznL3sAg0hO8fRb1mrCU1wIEYiA== X-Received: by 2002:a05:600c:3b10:b0:450:cf2e:7c92 with SMTP id 5b1f17b1804b1-450d885e46amr62054005e9.16.1748708498035; Sat, 31 May 2025 09:21:38 -0700 (PDT) Received: from [192.168.0.15] (cpc92320-cmbg19-2-0-cust719.5-4.cable.virginm.net. [82.13.66.208]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-450d8000d5dsm54834245e9.26.2025.05.31.09.21.37 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 31 May 2025 09:21:37 -0700 (PDT) Message-ID: Date: Sat, 31 May 2025 17:21:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: Content-Language: en-US From: Mark Thompson In-Reply-To: Subject: Re: [FFmpeg-devel] The "bad" Patch X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 31/05/2025 12:44, softworkz . wrote: >> -----Original Message----- >> From: ffmpeg-devel On Behalf Of softworkz . >> Sent: Donnerstag, 29. Mai 2025 04:59 >> To: FFmpeg development discussions and patches >> Subject: Re: [FFmpeg-devel] The "bad" Patch > > > Two and a half days have passed and nobody has answered any of the questions > from my previous post. This speaks for itself. > > What really happened, is that some had seen my use of system() and > since this is commonly known as a "dangerous" API when not used carefully, > they did lazy judgement without due diligence and without detailed > assessment. > The judgement was based on commonplace knowledge instead: > system() => is-bad => patch is bad > > > Looking at the specific case and way of usage (and how system() works > internally) would have revealed that this doesn't apply here. > > => The patch was NOT bad at all > => All review comments were addressed when it was pushed > > (the second part is evident anyway) The actions being taken in your patch (including but not limited to calling system()) were dangerous and the patch failed to prevent exploitation of the new features. I think the point you are missing boils down to: what is the threat model of ffmpeg? Your patch is built on the assumption that ffmpeg will only ever be run on single-user systems where many things can be trusted, but this is not the case. In reality, ffmpeg is often used on multi-user systems and called in strange ways from network services where many inputs are not trusted. In particular: * ffmpeg cannot trust anything which does not come from a known-trusted source (in practice this means it can trust little beyond its own binary). * ffmpeg must not assume that it is the only user on the system (anything which can be touched by another user at runtime may be modified). * ffmpeg should not in general trust the environment (it may be launched from a service which has allowed nasty things to be injected into the environment), though this may be allowed in specific well-known cases. * ffmpeg should not excessively trust its command-line arguments, as the caller may not have properly vetted them (in practice some nasty cases have to be allowed for the program to work at all). The main thing that it must prevent is any sort of code execution in the context of the current user, whether that be binary code in the current process or by launching another process running code which came from an untrusted source. Your patch was dangerous in at least the following ways: * It interacts with temporary directories in unsafe ways. * It trusts values coming from the environment. * It runs a shell with arguments which could have come from an untrusted source. The last one of those is completely unacceptable and I believe it lead people to reject the patch out of hand without considering anything else. Some example exploits: * Another user on a shared system observes the user of the common temporary directory. They create a file with the predictable name before your code does, then manipulate its contents. The command to open it then runs code under the attacker's control as the victim user. * The attacker manipulates the environment to inject their own code into the call to system(), running shell commands as the victim user (I gave a specific example of this in a previous email). This list is not exhaustive at all, so please do not treat it as a set of specific things to fix - there are very likely other paths for the attacker to achieve code execution in the context of the current user that I am not sufficiently clever to think of as the patch is not written in a secure and robust way. More generally, I am unsure whether anyone here believes themselves competent to review a patch which securely and robustly performs these sorts of very dangerous actions (I do not believe myself to be), so in practice ffmpeg developers will prefer to never include any code which performs such actions. Thanks, - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".