oshazard / apacman

ArchLinux User Repository (AUR) helper and pacman wrapper
https://aur.archlinux.org/packages/apacman/
GNU General Public License v3.0
70 stars 11 forks source link

enable --preview by default #8

Closed rmarquis closed 1 year ago

rmarquis commented 9 years ago

Since this fork seems to be better maintained than the upstream packer, you might be interested in the following.

Packer/apacman currently source PKGBUILDs before the user has a chance to view it, unless the --preview option is passed on. This is an important security flaw in the original design that can be fixed by enabling the --preview option by default or making it mandatory.

On the longer term, you might want to rework PKGBUILD parsing to enhance apacman usability while still ensuring security.

oshazard commented 9 years ago

@rmarquis Yes, keenerd seems to have mostly abandoned packer

While I agree sourcing unverified PKGBUILDs is a potential problem, parsing the contents manually runs the risk of dealing with a never-ending stream malformed edge cases.

Regarding a mandatory preview, hand-holding users is not a good practice. Flags can be set in the /etc/apacman.conf configuration file or an alias set at a users discretion. Trying to prevent users from using poor security practices is one reason many of the AUR helpers "broke" with the recent changes to makepkg in pacman-4.2.

I don't mean to sound elitist but ArchLinux is not a distro for novices. The lack of a review process is why the AUR is not officially supported by ArchLinux, which is a shame because its the distro's greatest asset IMO. A reminder of the official policy regarding AUR packages:

Unsupported packages are user produced content. Any use of the provided files is at your own risk.

I'd also like to add that apacman tries to follow the behaviour and coding style of packer as much as possible. That being said, rewriting the PKGBUILD handling is inevitable and will occur after the AUR revamp to git.

Finally I was tempted to mark this as wontfix but instead, I'll call for a vote on --preview enabled by default.

rmarquis commented 9 years ago

I don't mean to sound elitist but ArchLinux is not a distro for novices.

I understand this and fully agree with this statement, but this issue goes well beyond "spoon feeding" users imho. This security flaw simply defeats the purpose of reviewing PKGBUILD by the user, and in the current state, not using --preview is the very same as never checking PKGBUILD.

Note I'm glad someone finally stepped over to maintain the good old packer, but I wouldn't recommend apacman as a good helper alternative as long as this isn't fixed.

oshazard commented 9 years ago

So you have your own very prominent AUR helper pacaur? It may have been prudent had you simply pointed to #103 and mentioned that you fixed the issue long ago.

I made some progress towards this issue in apacman v1.5 (344849bea6e14a94e7615e998e275532ae45c4fb). These changes are enabled with --nosource flag, which selects and sanitizes the necessary variables from the PKGBUILDs.

I'd appreciate if you would look over what I have so far - I'm sure it will break for a number of special package types: split PKGBUILDs, conditional arrays, etc. Ultimately will need to either account for all conceivable edge cases or use a dedicated parser.

rmarquis commented 9 years ago

Pacaur is kinda irrelevant here. I'm more interested in signaling a security issue that isn't well known by packer/apacman userbase.

The new option is a good step, and yes it will likely break with all kind of bashism. You might want to check yaourt code too, since it does some tricks to avoid sourcing PKGBUILD directly with more or less success.

The way I solved this issue in pacaur is by using the AUR RPC interface instead of sourcing for dependency information. Unfortunately, implementing the same in apacman would probably means throwing away a big part of the code and starting from scratch, especially for better split packages support.

AladW commented 7 years ago

All this is trivial since the arrival of SRCINFO in 2013. Just copy what I suggested here: https://github.com/floft/spinach/issues/2