Re: Should creating a new base type require superuser status?

[Kris Jurka]

On Wed, 30 Jul 2008, Alvaro Herrera wrote:

> I think being able to return cstring from a user defined function is
> quite dangerous already.  I doubt we would ever give that capability to
> non-superusers.
>
> I do agree that creating base types should require a superuser though.
> It too seems dangerous just on principle, even if today there's no
> actual hole (that we already know of).

pl/java already allows non-superusers to create functions returning
cstring and base types built off of these functions.  It seems safe to me
if pl/java is doing the construction of cstring from a user provided
java.lang.String.

http://wiki.tada.se/display/pljava/Creating+a+Scalar+UDT+in+Java

Kris Jurka
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Tom Lane-2]

Kris Jurka <books@...> writes:
> On Wed, 30 Jul 2008, Alvaro Herrera wrote:
>> I do agree that creating base types should require a superuser though.
>> It too seems dangerous just on principle, even if today there's no
>> actual hole (that we already know of).

> pl/java already allows non-superusers to create functions returning
> cstring and base types built off of these functions.

So in other words, if pl/java is installed we have a security hole
a mile wide.

                        regards, tom lane
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Thomas Hallgren]

It seems perfectly safe to me too for the reason that Kris mentions.

Tom, could you please elaborate where you see a security hole?

Regards,
Thomas Hallgren

Tom Lane wrote:

> Kris Jurka <books@...> writes:
>  
>> On Wed, 30 Jul 2008, Alvaro Herrera wrote:
>>    
>>> I do agree that creating base types should require a superuser though.
>>> It too seems dangerous just on principle, even if today there's no
>>> actual hole (that we already know of).
>>>      
>
>  
>> pl/java already allows non-superusers to create functions returning
>> cstring and base types built off of these functions.
>>    
>
> So in other words, if pl/java is installed we have a security hole
> a mile wide.
>
> regards, tom lane
> _______________________________________________
> Pljava-dev mailing list
> Pljava-dev@...
> http://pgfoundry.org/mailman/listinfo/pljava-dev
>  

_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Tom Lane-2]

Thomas Hallgren <thomas@...> writes:
> Tom, could you please elaborate where you see a security hole?

The problem that we've seen in the past shows up when the user lies in
the CREATE TYPE command, specifying type representation properties that
are different from what the underlying functions expect.  In particular,
if it's possible to pass a pass-by-value integer to a function
that's expecting a pass-by-reference datum, you can misuse the function
to access backend memory.

I gather from looking at the example that Kris referenced that there's
some interface code in between the SQL function call and the user's Java
code, and that that interface code is itself looking at the declared
properties of the SQL type to decide what to do.  So to the extent that
that code is (a) bulletproof against inconsistencies and (b) not
subvertible by the PL/Java user, it might be that there's no hole in
practice.  But assumption (b) seems pretty fragile to me.

                        regards, tom lane
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Thomas Hallgren]

Tom Lane wrote:

> Thomas Hallgren <thomas@...> writes:
>  
>> Tom, could you please elaborate where you see a security hole?
>>    
>
> The problem that we've seen in the past shows up when the user lies in
> the CREATE TYPE command, specifying type representation properties that
> are different from what the underlying functions expect.  In particular,
> if it's possible to pass a pass-by-value integer to a function
> that's expecting a pass-by-reference datum, you can misuse the function
> to access backend memory.
>
>  

This is a non-issue in PL/Java. An integer parameter is never passed by
reference and there's no way the PL/Java user can get direct access to
backend memory.

> I gather from looking at the example that Kris referenced that there's
> some interface code in between the SQL function call and the user's Java
> code, and that that interface code is itself looking at the declared
> properties of the SQL type to decide what to do.  So to the extent that
> that code is (a) bulletproof against inconsistencies and (b) not
> subvertible by the PL/Java user, it might be that there's no hole in
> practice.  But assumption (b) seems pretty fragile to me.
>
>  
I think that assumption is without ground. Java doesn't permit you to
access memory unless you use Java classes (java.nio stuff) that is
explicitly designed to do that and you need native code to set such
things up. A PL/Java user can not do that unless he is able to link in
other shared objects or dll's to the backend process.

Based on that, I claim that your statement about a "security hole a mile
wide" is incorrect. PL/Java is not subject to issues relating to misuse
of backend memory.

Regards,
Thomas Hallgren

_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Tom Lane-2]

Thomas Hallgren <thomas@...> writes:
> Tom Lane wrote:
>> The problem that we've seen in the past shows up when the user lies in
>> the CREATE TYPE command, specifying type representation properties that
>> are different from what the underlying functions expect.

> This is a non-issue in PL/Java. An integer parameter is never passed by
> reference and there's no way the PL/Java user can get direct access to
> backend memory.

So what exactly does happen when the user deliberately specifies wrong
typlen/typbyval/typalign info when creating a type based on PL/Java
functions?

> I think that assumption is without ground. Java doesn't permit you to
> access memory unless you use Java classes (java.nio stuff) that is
> explicitly designed to do that and you need native code to set such
> things up. A PL/Java user can not do that unless he is able to link in
> other shared objects or dll's to the backend process.

PL/Java itself must be doing "unsafe" things in order to interface with
PG at all.  So what your argument really is is that you have managed to
securely sandbox the user-written code you are calling.  That might or
might not be true, but I don't think that worrying about it is without
foundation.

                        regards, tom lane
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Thomas Hallgren]

Tom Lane wrote:

>
>> This is a non-issue in PL/Java. An integer parameter is never passed by
>> reference and there's no way the PL/Java user can get direct access to
>> backend memory.
>>    
>
> So what exactly does happen when the user deliberately specifies wrong
> typlen/typbyval/typalign info when creating a type based on PL/Java
> functions?
>
>  

Everything is converted into instances of Java classes such as String,
byte[], etc.

>> I think that assumption is without ground. Java doesn't permit you to
>> access memory unless you use Java classes (java.nio stuff) that is
>> explicitly designed to do that and you need native code to set such
>> things up. A PL/Java user can not do that unless he is able to link in
>> other shared objects or dll's to the backend process.
>>    
>
> PL/Java itself must be doing "unsafe" things in order to interface with
> PG at all.  So what your argument really is is that you have managed to
> securely sandbox the user-written code you are calling.  That might or
> might not be true, but I don't think that worrying about it is without
> foundation.
>
>  

I would be presumptuous to claim that I provide the sandbox. All PL/Java
does is to provide the type mapping. The sandbox as such is implicit in
Java, much in the same way that it does it for web-browsers etc.

Regardless of that, I think there's some difference in expressing a
worry that might or might not have a foundation versus claiming that
there indeed must be a security hole a mile wide ;-)

- thomas

_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Kris Jurka]

On Sat, 2 Aug 2008, Tom Lane wrote:

> So what exactly does happen when the user deliberately specifies wrong
> typlen/typbyval/typalign info when creating a type based on PL/Java
> functions?

I have reviewed pljava's handling of misrepresented alignment, length, and
by value parameters

1) Alignment: pljava reads and writes data a byte at a time, so all types
effectively have char alignment.  Reading an integer will read
four bytes out of memory and then put those together.  Therefore the
alignment cannot be misspecified.

2) Length: For fixed length types, pljava correctly detects trying to
read or write too much data and not supplying enough data on write.
Pljava does not correctly handle variable length types.  It should be
setting and reading the length header itself rather than leaving that up
to the user, but it is not.

3) By value: pljava does not correctly handle passed by value types
correctly, allowing access to random memory.

So yes, pljava has a security problem, but I still object to the statement
that no PL can do this securely.  I will work on fixing pljava, but I
request the change for superuser requirement for type creation be
reverted.  The fact that no PL currently does it correctly is not a reason
to prohibit a PL from doing it correctly.

Kris Jurka
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Thomas Hallgren]

Kris Jurka wrote:
>
> 3) By value: pljava does not correctly handle passed by value types
> correctly, allowing access to random memory.
>
This is simply not true. There's no way a Java developer can access
random memory through PL/Java.

- thomas
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Kris Jurka]

Thomas Hallgren wrote:
> Kris Jurka wrote:
>>
>> 3) By value: pljava does not correctly handle passed by value types
>> correctly, allowing access to random memory.
>>
> This is simply not true. There's no way a Java developer can access
> random memory through PL/Java.

No, the point is that the Java developer can provide some data which can
convince postgresql to fetch random data for the user.

Consider the attached type which is simply an int4 equivalent.
Depending on how you define it as passed by value or passed by reference
it will or will not work (attached).

This looks like it works:


jurka=# select '1'::intbyref, '2'::intbyval;
  intbyref | intbyval
----------+----------
  1        | 2
(1 row)

But it doesn't really:

jurka=# create table inttest (a intbyref, b intbyval);
CREATE TABLE
jurka=# insert into inttest values ('1', '2');
INSERT 0 1
jurka=# select * from inttest;
  a |     b
---+------------
  1 | 2139062143
(1 row)

You can also get:

jurka=# select * from inttest;
server closed the connection unexpectedly
         This probably means the server terminated abnormally
         before or while processing the request.


Kris Jurka

package types;

import java.io.IOException;
import java.sql.SQLData;
import java.sql.SQLException;
import java.sql.SQLInput;
import java.sql.SQLOutput;
import java.util.logging.Logger;

public class Int implements SQLData
{
        private static Logger s_logger = Logger.getAnonymousLogger();

        private int m_i;
        private String m_typeName;

        public static Int parse(String input, String typeName) throws SQLException
        {
                try
                {
                        int i = Integer.parseInt(input);
                        return new Int(i, typeName);
                }
                catch(NumberFormatException e)
                {
                        throw new SQLException(e.getMessage());
                }
        }

        public Int()
        {
        }

        public Int(int i, String typeName)
        {
                m_i = i;
                m_typeName = typeName;
        }

        public String getSQLTypeName()
        {
                return m_typeName;
        }

        public void readSQL(SQLInput stream, String typeName) throws SQLException
        {
                s_logger.info(typeName + " from SQLInput");
                m_i = stream.readInt();
                m_typeName = typeName;
        }

        public void writeSQL(SQLOutput stream) throws SQLException
        {
                s_logger.info(m_typeName + " to SQLOutput");
                stream.writeInt(m_i);
        }

        public String toString()
        {
                s_logger.info(m_typeName + " toString");
                return Integer.toString(m_i);
        }

}

CREATE TYPE intbyval;

CREATE FUNCTION intbyval_in(cstring)
        RETURNS intbyval
        AS 'UDT[types.Int] input'
        LANGUAGE java IMMUTABLE STRICT;

CREATE FUNCTION intbyval_out(intbyval)
        RETURNS cstring
        AS 'UDT[types.Int] output'
        LANGUAGE java IMMUTABLE STRICT;

CREATE FUNCTION intbyval_recv(internal)
        RETURNS intbyval
        AS 'UDT[types.Int] receive'
        LANGUAGE java IMMUTABLE STRICT;

CREATE FUNCTION intbyval_send(intbyval)
        RETURNS bytea
        AS 'UDT[types.Int] send'
        LANGUAGE java IMMUTABLE STRICT;

CREATE TYPE intbyval (
        internallength = 4,
        input = intbyval_in,
        output = intbyval_out,
        receive = intbyval_recv,
        send = intbyval_send,
        passedbyvalue
        );

CREATE TYPE intbyref;

CREATE FUNCTION intbyref_in(cstring)
        RETURNS intbyref
        AS 'UDT[types.Int] input'
        LANGUAGE java IMMUTABLE STRICT;

CREATE FUNCTION intbyref_out(intbyref)
        RETURNS cstring
        AS 'UDT[types.Int] output'
        LANGUAGE java IMMUTABLE STRICT;

CREATE FUNCTION intbyref_recv(internal)
        RETURNS intbyref
        AS 'UDT[types.Int] receive'
        LANGUAGE java IMMUTABLE STRICT;

CREATE FUNCTION intbyref_send(intbyref)
        RETURNS bytea
        AS 'UDT[types.Int] send'
        LANGUAGE java IMMUTABLE STRICT;

CREATE TYPE intbyref (
        internallength = 4,
        input = intbyref_in,
        output = intbyref_out,
        receive = intbyref_recv,
        send = intbyref_send
        );


_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Thomas Hallgren]

Kris Jurka wrote:

> Thomas Hallgren wrote:
>> Kris Jurka wrote:
>>>
>>> 3) By value: pljava does not correctly handle passed by value types
>>> correctly, allowing access to random memory.
>>>
>> This is simply not true. There's no way a Java developer can access
>> random memory through PL/Java.
>
> No, the point is that the Java developer can provide some data which
> can convince postgresql to fetch random data for the user.
>
> Consider the attached type which is simply an int4 equivalent.
> Depending on how you define it as passed by value or passed by
> reference it will or will not work (attached).
>
> This looks like it works:
>
>
> jurka=# select '1'::intbyref, '2'::intbyval;
>  intbyref | intbyval
> ----------+----------
>  1        | 2
> (1 row)
>
> But it doesn't really:
>
> jurka=# create table inttest (a intbyref, b intbyval);
> CREATE TABLE
> jurka=# insert into inttest values ('1', '2');
> INSERT 0 1
> jurka=# select * from inttest;
>  a |     b
> ---+------------
>  1 | 2139062143
> (1 row)

It seems the pointer is confused for the actual value which means that
writing the value back will corrupt the pointer. That's bad of course
but I would classify this as a bug rather then a general security problem.

PL/Java is designed to do handle all types securely and completely hide
the concept of 'by value' or 'by reference' from the Java developer
since such concepts are meaningless in Java.

- thomas
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev

[Kris Jurka]

On Wed, 18 Feb 2009, Kris Jurka wrote:

> I have reviewed pljava's handling of misrepresented alignment, length, and by
> value parameters [and it doesn't all work.]
>

I have fixed pljava to now correctly handle all of these being defined
incorrectly.  So a trusted language can be used to create type input and
output functions safely.  I think the restriction that only superusers can
create types should be reverted.

Kris Jurka
_______________________________________________
Pljava-dev mailing list
Pljava-dev@...
http://pgfoundry.org/mailman/listinfo/pljava-dev