Bug#900920: stretch-pu: package freedink-dfarc/3.12-1+deb9u1
On 09/06/2018 22:29, Adam D. Barratt wrote: > On Fri, 2018-06-08 at 20:12 +0200, Sylvain wrote: >> On 08/06/2018 19:55, Adam D. Barratt wrote: >>> Control: tags -1 + confirmed >>> >>> On Wed, 2018-06-06 at 19:54 +0200, b...@debian.org wrote: Please consider this update to freedink-dfarc for stretch. It fixes a security issue that can overwrite arbitrary user files. Sending to stable following security team's directions from 2018- 06- 01. >>> +freedink-dfarc (3.12-1+deb9u1) stable; urgency=high >>> >>> Please use "stretch" as the distribution. >>> >>> + * Fix directory traversal in D-Mod extractor (CVE-2018-0496) >>> + * Upload to 'stable' as security team rejected a DSA to >>> +'stretch-security' (no justification) >>> >>> The changelog is not the place for such commentary - please remove >>> it. >>> >>> With the above changes made, and assuming that the resulting >>> package >>> has been tested on stretch, please feel free to upload. >> As per Social Contract #3 I do have to explain to my users why they >> get the security fix after the disclosure. > As with basically all core teams, Debian's security team is generally > stretched in terms of manpower and can't handle every possible update > that's security-related. Things have to be prioritised and sometimes > those updates end up being provided via proposed-updates. That's always > going to be the case in a volunteer project, and even larger and/or > commercially-backed projects will still have to decide which updates > they handle before others. This isn't a problem as such, just the way > things are. Workload: that's not what they say. When asked on IRC, they said the team was "fine". Priorities: I do accept them. However I can report that they are neither documented nor explained: - "In the past, uploads to |stable| were used to address security problems as well. However, this practice is deprecated" https://www.debian.org/doc/manuals/developers-reference/pkgs.en.html#upload-stable - "I don't think this warrants a DSA." is the sole explanation I could get. Plus, as I'm learning this 2-tier security support after years in Debian, I deemed this all-the-more relevant to the changelog. Incidentally, are you part of the Security Team? If yes, I'd appreciate that you say so. If not, that you don't speak for them. >> This is not a commentary, this is purely factual. > It's not a description of a change made to the package, nor information > that users need in order to decide whether they should be installing > it. As such, it is commentary. That has nothing to do with its > factuality or otherwise. It's a description of where the package is uploaded and why. Moreover I fail to see how adding this information is causing any harm, and in what way it's good to waste both our time complaining about it rather than just accepting the upload as-is. Since each question here needs a day or two to be answered, and since I'm not going to stall the update any more, I'll apply what will only look like helping hiding problems, as well as the AFAICS undocumented (https://www.debian.org/doc/manuals/developers-reference/pkgs.html#upload-stable) stable->stretch change. Working on Debian is so depressing these days. - Sylvain
Bug#900920: stretch-pu: package freedink-dfarc/3.12-1+deb9u1
On Fri, 2018-06-08 at 20:12 +0200, Sylvain wrote: > Hi, > > On 08/06/2018 19:55, Adam D. Barratt wrote: > > Control: tags -1 + confirmed > > > > On Wed, 2018-06-06 at 19:54 +0200, b...@debian.org wrote: > > > Please consider this update to freedink-dfarc for stretch. > > > It fixes a security issue that can overwrite arbitrary user > > > files. > > > Sending to stable following security team's directions from 2018- > > > 06- > > > 01. > > > > +freedink-dfarc (3.12-1+deb9u1) stable; urgency=high > > > > Please use "stretch" as the distribution. > > > > + * Fix directory traversal in D-Mod extractor (CVE-2018-0496) > > + * Upload to 'stable' as security team rejected a DSA to > > +'stretch-security' (no justification) > > > > The changelog is not the place for such commentary - please remove > > it. > > > > With the above changes made, and assuming that the resulting > > package > > has been tested on stretch, please feel free to upload. > > As per Social Contract #3 I do have to explain to my users why they > get the security fix after the disclosure. > As with basically all core teams, Debian's security team is generally stretched in terms of manpower and can't handle every possible update that's security-related. Things have to be prioritised and sometimes those updates end up being provided via proposed-updates. That's always going to be the case in a volunteer project, and even larger and/or commercially-backed projects will still have to decide which updates they handle before others. This isn't a problem as such, just the way things are. (There's an argument that co-ordinated disclosure is in fact hiding issues in and of itself. I don't particularly subscribe to that, nor do I believe that any of this is what SC3 is actually trying to ensure.) > This is not a commentary, this is purely factual. It's not a description of a change made to the package, nor information that users need in order to decide whether they should be installing it. As such, it is commentary. That has nothing to do with its factuality or otherwise. Regards, Adam
Bug#900920: stretch-pu: package freedink-dfarc/3.12-1+deb9u1
Hi, On 08/06/2018 19:55, Adam D. Barratt wrote: > Control: tags -1 + confirmed > > On Wed, 2018-06-06 at 19:54 +0200, b...@debian.org wrote: >> Please consider this update to freedink-dfarc for stretch. >> It fixes a security issue that can overwrite arbitrary user files. >> Sending to stable following security team's directions from 2018-06- >> 01. > +freedink-dfarc (3.12-1+deb9u1) stable; urgency=high > > Please use "stretch" as the distribution. > > + * Fix directory traversal in D-Mod extractor (CVE-2018-0496) > + * Upload to 'stable' as security team rejected a DSA to > +'stretch-security' (no justification) > > The changelog is not the place for such commentary - please remove it. > > With the above changes made, and assuming that the resulting package > has been tested on stretch, please feel free to upload. As per Social Contract #3 I do have to explain to my users why they get the security fix after the disclosure. This is not a commentary, this is purely factual. Please advise. - Sylvain
Processed: Re: Bug#900920: stretch-pu: package freedink-dfarc/3.12-1+deb9u1
Processing control commands: > tags -1 + confirmed Bug #900920 [release.debian.org] stretch-pu: package freedink-dfarc/3.12-1+deb9u1 Added tag(s) confirmed. -- 900920: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=900920 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#900920: stretch-pu: package freedink-dfarc/3.12-1+deb9u1
Control: tags -1 + confirmed On Wed, 2018-06-06 at 19:54 +0200, b...@debian.org wrote: > Please consider this update to freedink-dfarc for stretch. > It fixes a security issue that can overwrite arbitrary user files. > Sending to stable following security team's directions from 2018-06- > 01. +freedink-dfarc (3.12-1+deb9u1) stable; urgency=high Please use "stretch" as the distribution. + * Fix directory traversal in D-Mod extractor (CVE-2018-0496) + * Upload to 'stable' as security team rejected a DSA to +'stretch-security' (no justification) The changelog is not the place for such commentary - please remove it. With the above changes made, and assuming that the resulting package has been tested on stretch, please feel free to upload. Regards, Adam
Bug#900920: stretch-pu: package freedink-dfarc/3.12-1+deb9u1
Package: release.debian.org
User: release.debian@packages.debian.org
Usertags: pu
Tags: stretch
Severity: normal
Hi,
Please consider this update to freedink-dfarc for stretch.
It fixes a security issue that can overwrite arbitrary user files.
Sending to stable following security team's directions from 2018-06-01.
Attached is the debdiff.
Let me know if I can push the upload to stable.
Cheers!
Sylvain
diff -Nru freedink-dfarc-3.12/debian/changelog
freedink-dfarc-3.12/debian/changelog
--- freedink-dfarc-3.12/debian/changelog2014-10-16 23:04:34.0
+0200
+++ freedink-dfarc-3.12/debian/changelog2018-06-05 21:41:38.0
+0200
@@ -1,3 +1,11 @@
+freedink-dfarc (3.12-1+deb9u1) stable; urgency=high
+
+ * Fix directory traversal in D-Mod extractor (CVE-2018-0496)
+ * Upload to 'stable' as security team rejected a DSA to
+'stretch-security' (no justification)
+
+ -- Sylvain Beucler Tue, 05 Jun 2018 21:41:38 +0200
+
freedink-dfarc (3.12-1) unstable; urgency=medium
* New Upstream Release
diff -Nru freedink-dfarc-3.12/debian/patches/CVE-2018-0496.patch
freedink-dfarc-3.12/debian/patches/CVE-2018-0496.patch
--- freedink-dfarc-3.12/debian/patches/CVE-2018-0496.patch 1970-01-01
01:00:00.0 +0100
+++ freedink-dfarc-3.12/debian/patches/CVE-2018-0496.patch 2018-06-05
21:41:38.0 +0200
@@ -0,0 +1,232 @@
+Description: Fix directory traversal in D-Mod extrator (CVE-2018-0496)
+Author: Sylvain Beucler
+Origin: upstream
+Last-Update: 2018-06-05
+
+commit 40cc957f52e772f45125126439ba9333cf2d2998
+Author: Sylvain Beucler
+Date: Tue Jun 5 23:07:56 2018 +0200
+
+Fix directory traversal in D-Mod extractor (CVE-2018-0496)
+
+diff --git a/src/InstallVerifyFrame.cpp b/src/InstallVerifyFrame.cpp
+index 64964e1..494ba53 100644
+--- a/src/InstallVerifyFrame.cpp
b/src/InstallVerifyFrame.cpp
+@@ -3,7 +3,7 @@
+
+ * Copyright (C) 2004 Andrew Reading
+ * Copyright (C) 2005, 2006 Dan Walma
+- * Copyright (C) 2008 Sylvain Beucler
++ * Copyright (C) 2008, 2018 Sylvain Beucler
+
+ * This file is part of GNU FreeDink
+
+@@ -55,7 +55,10 @@ InstallVerifyFrame::InstallVerifyFrame(const wxString&
lDmodFilePath)
+ {
+ // Prepare the tar file for reading
+ Tar lTar(mTarFilePath);
+- lTar.ReadHeaders();
++ if (lTar.ReadHeaders() == 1) {
++this->EndModal(wxID_CANCEL);
++return;
++ }
+
+ // Get and display the dmod description
+ wxString lDmodDescription = lTar.getmDmodDescription();
+@@ -122,7 +125,20 @@ void InstallVerifyFrame::onInstall(wxCommandEvent )
+ destdir = mConfig->mDModDir;
+
+ Tar lTar(mTarFilePath);
+- lTar.ReadHeaders();
++ if (lTar.ReadHeaders() == 1) {
++this->EndModal(wxID_CANCEL);
++return;
++ }
++
++ if (wxDirExists(destdir + wxFileName::GetPathSeparator() +
lTar.getInstalledDmodDirectory())) {
++wxString question;
++question.Printf(_("Directory '%s' already exists. Continue?"),
lTar.getInstalledDmodDirectory());
++int lResult = wxMessageBox(question, _("DFArc - Installing"),
++ wxYES_NO | wxICON_WARNING, this);
++if (lResult == wxNO)
++ return;
++ }
++
+ int lError = lTar.Extract(destdir, );
+ if (lError == 0)
+ {
+diff --git a/src/Tar.cpp b/src/Tar.cpp
+index b919ae7..8147b8a 100644
+--- a/src/Tar.cpp
b/src/Tar.cpp
+@@ -3,7 +3,7 @@
+
+ * Copyright (C) 2004 Andrew Reading
+ * Copyright (C) 2005, 2006 Dan Walma
+- * Copyright (C) 2008, 2014 Sylvain Beucler
++ * Copyright (C) 2008, 2014, 2018 Sylvain Beucler
+
+ * This file is part of GNU FreeDink
+
+@@ -31,6 +31,7 @@
+ #include
+ #include
+ #include
++#include
+
+ #include
+ #include
+@@ -427,7 +428,15 @@ int Tar::ReadHeaders( void )
+ wxString lPath(lRecord.Name, wxConvUTF8);
+ if (mInstalledDmodDirectory.Length() == 0)
+ {
+-mInstalledDmodDirectory = lPath.SubString( 0, lPath.Find( '/' ) );
++// Security: ensure the D-Mod directory is non-empty
++wxString firstDir = GetFirstDir(lPath);
++if (firstDir.IsSameAs("", true) || firstDir.IsSameAs("..", true) ||
firstDir.IsSameAs("dink", true))
++{
++wxLogError(_("Error: invalid D-Mod directory. Stopping."));
++ return 1;
++}
++ mInstalledDmodDirectory = firstDir;
++
+ lDmodDizPath = mInstalledDmodDirectory + _T("dmod.diz");
+ lDmodDizPath.LowerCase();
+ }
+@@ -472,10 +481,6 @@ int Tar::Extract(wxString destdir, wxProgressDialog*
aProgressDialog)
+ wxString strBuf;
+ int lError = 0;
+
+-// Remember current directory
+-wxString strCwd = ::wxGetCwd();
+-
+-
+ // Open the file here so it doesn't error after changing.
+ wxFile wx_In(mFilePath, wxFile::read);
+
+@@ -495,8 +500,6 @@ int Tar::Extract(wxString destdir, wxProgressDialog*
aProgressDialog)
+ wxLogFatalError(_("Error: Cannot create directory '%s'. Cannot
