Issues about SQLite archive files

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Issues about SQLite archive files

Simon Slavin-3
<https://www.sqlite.org/draft/sqlar.html>

One important security issue and several unimportant ones.

Security issue:

The built-in path functions of some operating systems interpret file paths beginning with a path separator as relative to the root of the drive rather than the current subdirectory.  For instance

    tmp/read.me <-- subdirectory called 'tmp' of default path
    /tmp/read.me <-- subdirectory called 'tmp' of the default drive's root

A 'name' field in a SQLite archive file could be edited to hold such a path.  This would be a vulnerability if a utility used to 'unpack' an archive is accidentally given too much power, as 'install' scripts often are.  I would suggest some attention be paid to this issue in the command-line tools which understand archives, that built-in functions which store the 'name' field not store leading path separators, and that functions which restore from the 'name' field filter out leading path separators.  This could be done with a single 'sqlar_sanitizeName()' function (you can probably think of a better name for it) called by both packer and expander functions.

You might also want to ponder whether a 'name' starting with 'c:\' should be allowed, and what the command-line tools should do about it if it isn't.

Other issues:

Because not all operating systems use the same character as a subdirectory separator, you might document whether archives always use the '/' character as a path separator.  Proper programmers know what they're doing, but naive Windows users might not.

Is there a good reason for the 'sz' column to be called 'sz' and not 'size' or 'length' ?  I dislike pointless abbreviation.

If you're renaming the 'sz' field, please consider whether to rename 'name' to 'path', which is closer to the industry standard.

Does the deflated stream include a header which says "what follows is deflated data" ?  If not, what will you do when users want to use better algorithms than 'Deflate' ?  You need either a column which tells the user which compression method was used (including 'none') or a function which extracts this information from the stored BLOB.

If you expect SQLite archive files to eventually feature other methods of compression than 'Inflate', the documentation should make it clear whether this would be done by changing sqlar.c or by supplying other files like sqlar.c.  In the latter case, the name and functions of sqlar.c should be changed to reflect that they are the 'Inflate' versions.

Simon.
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Issues about SQLite archive files

Jens Alfke-2


> On Mar 15, 2018, at 1:26 PM, Simon Slavin <[hidden email]> wrote:
>
> The built-in path functions of some operating systems interpret file paths beginning with a path separator as relative to the root of the drive rather than the current subdirectory.  […]
> A 'name' field in a SQLite archive file could be edited to hold such a path.  This would be a vulnerability if a utility used to 'unpack' an archive is accidentally given too much power, as 'install' scripts often are.

It’s pretty common for install scripts to write to absolute paths, since many types of installable components need to go in specific locations in the filesystem. Any time you run an installer you’re implicitly giving it permission to write anywhere in the filesystem that you can access; fortunately the really sensitive locations (/etc, /System/Library, etc.) are locked down to root accounts.

But for the general purpose extraction CLI, it’d be wise to either fail if an absolute path is found, or allow absolute paths only if a special flag is given.

> Because not all operating systems use the same character as a subdirectory separator, you might document whether archives always use the '/' character as a path separator.

+1. If sqlar is to be a cross-platform archive format like Zip, it should have a single format for representing paths.

> Does the deflated stream include a header which says "what follows is deflated data" ?  If not, what will you do when users want to use better algorithms than 'Deflate’ ?

It looks as though it doesn’t. The discussion of sqlar_uncompress() shows that it tells compressed data apart from uncompressed by comparing the data size against the original file size, which would be unnecessary if there were metadata identifying the compression algorithm.

I agree it would be a good idea to add such metadata, either in the blob itself or as a table column. The ‘deflate’ algorithm is pretty long in the tooth, and newer algorithms like lz4 and zstd offer much better compression and performance. It would be wise to future-proof the archive format to allow for them.

—Jens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users