| Age | Commit message (Collapse) | Author |
|---|
|
-1 is not an allowed value for the file descriptor, the only
allowed non-file-descriptor value is AT_FDCWD. So use that
instead.
AT_SYMLINK_NOFOLLOW has a weird semantic: It checks whether
we have the specified access on the symbolic link. It also
is implemented only by glibc on Linux, so it's inherently
non-portable. We should just drop it.
Thanks: James Clarke for debugging these issues
Reported-by: James Clarke <jrtc27@jrtc27.com>
(cherry picked from commit 25f54c960d7a4ceca7bd3e21f87baf48d6cbc2d3)
|
|
apt tools do not really support these other variables, but tools apt
calls might, so lets play save and clean those up as needed.
Reported-By: Paul Wise (pabs) on IRC
(cherry picked from commit e2c8c825a5470e33c25d00e07de188d0e03922c8)
|
|
We can't cleanup the environment like e.g. sudo would do as you usually
want the environment to "leak" into these helpers, but some variables
like HOME should really not have still the value of the root user – it
could confuse the helpers (USER) and HOME isn't accessible anyhow.
Closes: 842877
(cherry picked from commit 34b491e735ad47c4805e63f3b83a659b8d10262b)
|
|
This is a follow up to the previous issue where we did not check
if getline() returned -1 due to an end of file or due to an error
like memory allocation, treating both as end of file.
Here we ensure that we also handle buffered writes correctly by
flushing the files before checking for any errors in our error
stack.
Buffered writes themselves were introduced in 1.1.9, but the
function was never called with a buffered file from inside
apt until commit 46c4043d741cb2c1d54e7f5bfaa234f1b7580f6c
which was first released with apt 1.2.10. The function is
public, though, so fixing this is a good idea anyway.
Affected: >= 1.1.9
(cherry picked from commit 6212ee84a517ed68217429022bd45c108ecf9f85)
|
|
This fixes a security issue where signatures of the
InRelease files could be circumvented in a man-in-the-middle
attack, giving attackers the ability to serve any packages
they want to a system, in turn giving them root access.
It turns out that getline() may not only return EINVAL
as stated in the documentation - it might also return
in case of an error when allocating memory.
This fix not only adds a check that reading worked
correctly, it also implicitly checks that all writes
worked by reporting any other error that occurred inside
the loop and was logged by apt.
Affected: >= 0.9.8
Reported-By: Jann Horn <jannh@google.com>
Thanks: Jann Horn, Google Project Zero for reporting the issue
LP: #1647467
(cherry picked from commit 51be550c5c38a2e1ddfc2af50a9fab73ccf78026)
|
|
This fixes a regression introduced in
commit 8f858d560e3b7b475c623c4e242d1edce246025a
don't leak FD in AutoProxyDetect command return parsing
which accidentally made the proxy autodetection code also read
the scripts output on stderr, not only on stdout when it switched
the code from popen() to Popen().
Reported-By: Tim Small <tim@seoss.co.uk>
|
|
memcpy is marked as nonnull for its input, but ignores the input anyhow
if the declared length is zero. Our SHA2 implementations do this as
well, it was "just" MD5 and SHA1 missing, so we add the length check
here as well as along the callstack as it is really pointless to do all
these method calls for "nothing".
Reported-By: gcc -fsanitize=undefined
|
|
If the inner Base256ToNum() returned false, it did not set
Num to a new value, causing it to be uninitialized, and thus
might have caused the function to exit despite a good result.
Also document why the Res = Num, if (Res != Num) magic is done.
Reported-By: valgrind
|
|
This allows other vendors to use different paths, or to build
your own APT in /opt for testing. Note that this uses + 1 in
some places, as the paths we receive are absolute, but we need
to strip of the initial /.
|
|
The C.UTF-8 locale is not portable, so we need to use C, otherwise
we crash on other systems. We can use std::locale::classic() for
that, which might also be a bit cheaper than using locale("C").
|
|
Several modules use std::array without including the
array header. Bad modules.
Some modules use STDOUT_FILENO and friends, or close()
without including unistd.h, where they are defined.
One module also uses WIFEXITED() without including
sys/wait.h.
Finally, environ is not specified to be defined in unistd.h. We
are required to define it ourselves according to POSIX, so let's
do that.
|
|
Since its existence in 2010 DirectoryExists was always marked with this
attribute, but for no real reason. Arguably a check for the existence of
the file is not modifying global state, so theoretically this shouldn't
be a problem. It is wrong from a logical point of view through as
between two calls the directory could be created so the promise we made
to the compiler that it could remove the second call would be wrong, so
API wise it is wrong.
It's a bit mysterious that this is only observeable on ppc64el and can be
fixed by reordering code ever so slightly, but in the end its more our
fault for adding this attribute than the compilers fault for doing
something silly based on the attribute.
LP: 1473674
|
|
When checking if a file is empty, we forget to check that
fstat() actually worked.
|
|
We use clock() as a very cheap way of getting a "random" value, but the
manpage warns that this could return -1, so we should be dealing with
this. Additionally, e.g. on hurd-i386 the value increases only slowly –
to slow for our fast running tests for randomness hence producing the
same range in both samples, so we introduce a simple busy-wait loop (as
clock is counting processor time used by the program) in the test which
delays the second sample just enough making our randomness a bit more
predictable.
|
|
|
|
If the URI had no password the username was ignored
|
|
Socks support is a requested feature in sofar that the internet is
actually believing Acquire::socks::Proxy would exist. It doesn't and
this commit isn't adding it as that isn't how our configuration works,
but it allows Acquire::http::Proxy="socks5h://…". The HTTPS method was
changed already to support socks proxies (all versions) via curl. This
commit implements only SOCKS5 (RFC1928) with no auth or pass&user auth
(RFC1929), but not GSSAPI which is required by the RFC. The 'h' in the
protocol name further indicates that DNS resolution is delegated to the
socks proxy rather than performed locally.
The implementation works and was tested with Tor as socks proxy for
which implementing socks5h only can actually be considered a feature.
Closes: 744934
|
|
Create a temporary configuration file with a dump of our
configuration and pass that to apt-key.
LP: #1607283
|
|
|
|
Create a local exiter object which cleans up files on exit.
|
|
Previously, when data could be created and sig not, we would unlink
sig, not data (and vice versa).
|
|
There is no point in trying to perform Write/Read on a FileFd which
already failed as they aren't going to work as expected, so we should
make sure that they fail early on and hard.
|
|
Reported-By: cppcheck
Gbp-Dch: Ignore
|
|
The flush call is a no-op in most FileFd implementations so this isn't
as critical as it might sound as the only non-trivial implementation is
in the buffered writer, which tends not be used to buffer another
buffer…
|
|
Very unlikely, but if the parent is /dev/null, the child empty and the
grandchild a value we returned /dev/null/value which doesn't exist, so
hardly a problem, but for best operability we should be consistent in
our work and return /dev/null always.
|
|
If we have files in partial/ from a previous invocation or similar such
those could be symlinks created by file:// sources. The code is
expecting only real files through and happily changes owner,
modification times and permission on the file the symlink points to
which tend to be files we have no business in touching in this way.
Permissions of symlinks shouldn't be changed, changing owner is usually
pointless to, but just to be sure we pick the easy way out and use
lchown, check for symlinks before chmod/utimes.
Reported-By: Mattia Rizzolo on IRC
|
|
If libapt has builtin support for a compression type it will create a
dummy compressor struct with the Binary set to 'false' as it will catch
these before using the generic pipe implementation which uses the
Binary. The catching happens based on configured Names through, so you
can actually force apt to use the external binaries even if it would
usually use the builtin support. That logic fails through if you don't
happen to have these external binaries installed as it will fallback to
calling 'false', which will end in confusing 'Write error's.
So, this is again something you only encounter in constructed testing.
Gbp-Dch: Ignore
|
|
All apt versions support numeric as well as 3-character timezones just
fine and its actually hard to write code which doesn't "accidently"
accepts it. So why change? Documenting the Date/Valid-Until fields in
the Release file is easy to do in terms of referencing the
datetime format used e.g. in the Debian changelogs (policy §4.4). This
format specifies only the numeric timezones through, not the nowadays
obsolete 3-character ones, so in the interest of least surprise we should
use the same format even through it carries a small risk of regression
in other clients (which encounter repositories created with
apt-ftparchive).
In case it is really regressing in practice, the hidden option
-o APT::FTPArchive::Release::NumericTimezone=0
can be used to go back to good old UTC as timezone.
The EDSP and EIPP protocols use this 'new' format, the text interface
used to communicate with the acquire methods does not for compatibility
reasons even if none of our methods would be effected and I doubt any
other would (in these instances the timezone is 'GMT' as that is what
HTTP/1.1 requires). Note that this is only true for apt talking to
methods, (libapt-based) methods talking to apt will respond with the
'new' format. It is therefore strongly adviced to support both also in
method input.
|
|
We deploy atomic renames for some files, but these renames also happen
if something about the file failed which isn't really the point of the
exercise…
Closes: 828908
|
|
This reverts commit 2b8221d66a8284042fc53c7bbb14bb9750e9137f.
Avoiding the use of GCC >= 5 stuff lets use go back to 4.8 simplifying
the travis setup again as well as reducing the backport requirements in
general.
This is possible because the std::get_time use requiring GCC >= 5 in
9febc2b238e1e322dce1f94ecbed46d595893b52 was replaced by handrolling it
in 1d742e01470bba27715a8191c50adde4b39c2f19, so the remaining uses are
just small conviniences we can do without.
Gbp-Dch: Ignore
|
|
Rewritten in 9febc2b238e1e322dce1f94ecbed46d595893b52 for c++ locales
usage and rewritten again in 1d742e01470bba27715a8191c50adde4b39c2f19 to
avoid a currently present stdlibc++6 bug in the std::get_time
implementation. The later implementation uses still stringstreams for
parsing, but forgot to explicitly reset the locale to something sane
(for parsing english dates that is), so date and especially the parsing
of a number is depending on the locale. Turns out, the French (among
others) format their numbers with space as thousand separator so for
some reason the stdlibc++6 thinks its a good idea to interpret the
entire datetime string as a single number instead of realizing that in
"25 Jun …" the later parts can't reasonably be part of that number even
through there are spaces there…
Workaround is hence: LC_NUMERIC=C.UTF-8
Closes: 828011
|
|
There is a subtile difference between an empty setting and "DIRECT" in
the configuration as the later overrides the generic settings while the
earlier does not. Also, non-zero exitcodes should really be reported as
an error rather than silently discarded.
|
|
Regression introduced in 8f858d560e3b7b475c623c4e242d1edce246025a.
Commands are probably better of always having output through as the
fall through to the generic proxy settings is likely not intended. As
documenting and implementing this more consistently is kind of a
regression through, it is split off into the next commit.
Closes: 827713
|
|
As reported upstream in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71556
the implementation of std::get_time is currently not as accepting as
strptime is, especially in how hours should be formatted.
Just reverting 9febc2b238e1e322dce1f94ecbed46d595893b52 would be
possible, but then we would reopen the problems fixed by it, so instead
I opted here for a rewrite of the parsing logic which makes this method
a lot longer, but at least it provides the same benefits as the rewrite
in std::get_time was intended to give us and decouples us from the fix
of the issue in the standard library implementation of GCC.
LP: 1593583
|
|
We usually use absolute paths to specific the location of dpkg, apt-key
and the like, but there is nothing wrong with using just the command
name and instead let exec(3) make the lookup in PATH.
We had a wild mixture before, so opting for the more accepting option
out of the two seems about right especially as it makes no difference in
the default case as apt uses absolute paths.
|
|
Just closing the fd would be enough, but while we are at it we can also
use the Popen interface to have an easier time with this.
|
|
Seen first in #826783, but as this buglog also shows leaked uncompressed
files as well we don't close it just yet.
|
|
This effects only compressors configured on the fly (rather then the
inbuilt ones as they use a library).
|
|
|
|
HTTP/1.1 hardcodes GMT (RFC 7231 §7.1.1.1) and what is good enough for the
internet must be good enough for us™ as we reuse the implementation
internally to parse (most) dates we encounter in various places like the
Release files with their Date and Valid-Until header fields.
Implementing a fully timezone aware parser just feels too hard for no
effective benefit as it would take 5+ years (= until LTS's are out of
fashion) until a repository could use non-UTC dates and expect it to
work. Not counting non-apt implementations which might or might not
only want to encounter UTC here as well.
As a bonus, this eliminates the use of an instance of setlocale in
libapt.
Closes: 819697
|
|
Setting the C++ locale via std::locale::global(std::locale("")); which
would otherwise default to the default C locale (aka: unaffected by
setlocale) effects the formatting of numeric types in IO streams, which
for output for humans is perfectly sensible, but breaks our many text
interfaces used and parsed by us and others without expecting the
numbers to be formatted.
Closes: #825396
|
|
The &= introduced in the EDSP-FileFd conversion isn't working to full
satisfaction for multiple && clauses as the && has a higher binding than
&= has, so that the methods were called even through they shouldn't
have because of previous errors. Using variadic functions we can solve
this in a slightly cleaner way bringing down the amount of 'broken pipe'
errors for the error case of the dump resolver substantially.
Git-Dch: Ignore
|
|
I doubt there is any non-src:apt usage of these interfaces.
|
|
Improve GetLocalitySortedVersionSet, speeds up apt search by 30%
|
|
The actual reason for this commit isn't the limit – there isn't much
point in using that much nesting – its in shutting up gcc mostly:
apt/apt-pkg/contrib/configuration.cc: In function ‘bool ReadConfigFile(Configuration&, const string&, const bool&, const unsigned int&)’:
apt/apt-pkg/contrib/configuration.cc:686:20: warning: cannot optimize loop, the loop counter may overflow [-Wunsafe-loop-optimizations]
string Stack[100];
^
by replacing this with C++s handy std::stack container (adapter).
Also cleans some whitespace noise from the file in the process.
|
|
|
|
If the file is in a failed state there is no point in trying to flush
out the buffers as the file is to be discarded anyhow & its likely all
this flushing is producing is additional error messages.
Git-Dch: Ignore
|
|
|
|
Hardly noticeable, but given that we have the option to easily enable
it, lets enable it as every newline in the message is written
individually by the code.
|
|
Some methods had it missing, some used the keyword directly, which isn't
a problem as it is a cc file, but for consistency lets stick to our
macro for now.
Git-Dch: Ignore
|
