FS#23902 - [namcap] parsepkgbuild can hang on valid PKGBUILD

Attached to Project: Arch Linux
Opened by Dan McGee (toofishes) - Saturday, 23 April 2011, 18:32 GMT
Last edited by Rémy Oudompheng (remyoudompheng) - Monday, 25 April 2011, 10:56 GMT
Task Type Bug Report
Category Arch Projects
Status Closed
Assigned To Rémy Oudompheng (remyoudompheng)
Architecture All
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

$ cd arch-repos/postgresql/trunk/
$ namcap PKGBUILD

It will never return, getting stuck waiting for parsepkgbuild to return...which does not happen, due to this:

$ parsepkgbuild PKGBUILD
<snip>
PKGBUILD: line 100: cd: restricted
make: *** No rule to make target `install'. Stop.
make: *** contrib: No such file or directory. Stop.
make: *** doc/src/sgml: No such file or directory. Stop.
make: *** src/bin/pg_config: No such file or directory. Stop.
rm: cannot remove `/usr/share/man/man1/pg_config.1': No such file or directory
install: cannot stat `COPYRIGHT': No such file or directory
rm: cannot remove `/usr/include/postgresql/internal/libpq-int.h': Permission denied
rm: cannot remove `/usr/include/postgresql/internal/libpq/pqcomm.h': Permission denied
rm: cannot remove `/usr/include/postgresql/internal/port.h': Permission denied
rm: cannot remove `/usr/include/postgresql/internal/postgres_fe.h': Permission denied
rm: cannot remove `/usr/include/postgresql/internal/c.h': Permission denied
rm: cannot remove `/usr/include/postgresql/internal/pqexpbuffer.h': Permission denied
rm: cannot remove `/usr/include/libpq/libpq-fs.h': Permission denied
rm: remove write-protected regular file `./term.h'? ^C

Where it hangs.
This task depends upon

Closed by  Rémy Oudompheng (remyoudompheng)
Monday, 25 April 2011, 10:56 GMT
Reason for closing:  Fixed
Additional comments about closing:  To appear in upcoming namcap 3.1
Comment by Rémy Oudompheng (remyoudompheng) - Sunday, 24 April 2011, 08:46 GMT
This looks awfully dangerous. Seems like the parsing script is run with the default PATH.

What do you think of the attached patch?
   patch (1 KiB)
Comment by Dan McGee (toofishes) - Sunday, 24 April 2011, 19:47 GMT
The stdin bit makes sense, so +1 on that part.

However, the PATH setting works in this case, but still hides the problem here- we attempt to run everything and we shouldn't be doing so. Setting path doesn't help one bit if I am using shell builtins, absolute paths (/bin/rm), etc.
Comment by Rémy Oudompheng (remyoudompheng) - Sunday, 24 April 2011, 20:00 GMT
Absolute paths for programs are inhibited by the restricted shell mode. However I agree that executing a script in order to parse it does not make sense. But unless we restrict the shell constructs that are allowed to define variables in PKGBUILDs, writing a proper parser will almost need to reimplement bash.

What builtins are you thinking about which are not disabled by the restricted mode?
Comment by Dan McGee (toofishes) - Sunday, 24 April 2011, 20:07 GMT
I didn't see anything in the documentation that addressed what builtins are allowed by default, so I could be wrong. Now that you say (and I read) absolute path commands are not allowed, this patch makes sense, so +1 from me.

Loading...