Skip to content

Commit 7c83a6f

Browse files
timfeltimfel
committed
[hotfix] Security fixes for MLE
PullRequest: graalpython/252
2 parents 422a51b + 72517fd commit 7c83a6f

File tree

13 files changed

+278
-60
lines changed

13 files changed

+278
-60
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/__init__.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ def setUp(self):
7171

7272

7373
def ccompile(self, name):
74+
if getattr(sys, "graal_python_opaque_filesystem", False):
75+
# distutils won't fully work with an opaque filesystem,
76+
# because we cannot read bytes from files and manipulate
77+
# them. We hope the code was already compiled.
78+
return
79+
7480
from distutils.core import setup, Extension
7581
source_file = '%s/%s.c' % (__dir__, name)
7682
file_not_empty(source_file)
@@ -459,7 +465,7 @@ def CPyExtType(name, code, **kwargs):
459465
{nb_inplace_matrix_multiply},
460466
""" if sys.version_info.minor >= 6 else "") + """
461467
}};
462-
468+
463469
static struct PyMethodDef {name}_methods[] = {{
464470
{tp_methods},
465471
{{NULL, NULL, 0, NULL}}

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_err.py‎

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -152,23 +152,24 @@ def compile_module(self, name):
152152
cmpfunc=unhandled_error_compare
153153
)
154154

155-
test_PyErr_PrintEx = CPyExtFunction(
156-
lambda args: None,
157-
lambda: (
158-
(True,),
159-
),
160-
code="""PyObject* wrap_PyErr_PrintEx(int n) {
161-
PyErr_SetString(PyExc_KeyError, "unknown key whatsoever");
162-
PyErr_PrintEx(n);
163-
return Py_None;
164-
}
165-
""",
166-
resultspec="O",
167-
argspec='i',
168-
arguments=["int n"],
169-
callfunction="wrap_PyErr_PrintEx",
170-
cmpfunc=unhandled_error_compare
171-
)
155+
if not getattr(sys, "graal_python_opaque_filesystem", False):
156+
test_PyErr_PrintEx = CPyExtFunction(
157+
lambda args: None,
158+
lambda: (
159+
(True,),
160+
),
161+
code="""PyObject* wrap_PyErr_PrintEx(int n) {
162+
PyErr_SetString(PyExc_KeyError, "unknown key whatsoever");
163+
PyErr_PrintEx(n);
164+
return Py_None;
165+
}
166+
""",
167+
resultspec="O",
168+
argspec='i',
169+
arguments=["int n"],
170+
callfunction="wrap_PyErr_PrintEx",
171+
cmpfunc=unhandled_error_compare
172+
)
172173

173174
test_PyErr_GivenExceptionMatches = CPyExtFunction(
174175
_reference_givenexceptionmatches,
@@ -266,16 +267,17 @@ def compile_module(self, name):
266267
cmpfunc=unhandled_error_compare
267268
)
268269

269-
test_PyErr_WarnEx = CPyExtFunctionVoid(
270-
lambda args: warnings.warn(args[1], args[0], args[2]),
271-
lambda: (
272-
(UserWarning, "custom warning", 1),
273-
),
274-
resultspec="O",
275-
argspec='Osn',
276-
arguments=["PyObject* category", "char* msg", "Py_ssize_t level"],
277-
cmpfunc=unhandled_error_compare
278-
)
270+
if not getattr(sys, "graal_python_opaque_filesystem", False):
271+
test_PyErr_WarnEx = CPyExtFunctionVoid(
272+
lambda args: warnings.warn(args[1], args[0], args[2]),
273+
lambda: (
274+
(UserWarning, "custom warning", 1),
275+
),
276+
resultspec="O",
277+
argspec='Osn',
278+
arguments=["PyObject* category", "char* msg", "Py_ssize_t level"],
279+
cmpfunc=unhandled_error_compare
280+
)
279281

280282
test_PyErr_NoMemory = CPyExtFunctionVoid(
281283
_reference_nomemory,

‎graalpython/com.oracle.graal.python.test/src/tests/test_pyio.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,11 @@ def test_builtin_open():
176176
unlink(file_name)
177177

178178
assert success
179+
180+
181+
import sys
182+
if getattr(sys, "graal_python_opaque_filesystem", False):
183+
# this cannot possibly work with opaque files
184+
for k in globals():
185+
if k.startswith("test_"):
186+
del globals()[k]

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@
7474
import com.oracle.truffle.api.TruffleFile;
7575
import com.oracle.truffle.api.TruffleLanguage;
7676
import com.oracle.truffle.api.TruffleLogger;
77-
import com.oracle.truffle.api.TruffleOptions;
7877
import com.oracle.truffle.api.debug.DebuggerTags;
7978
import com.oracle.truffle.api.frame.Frame;
8079
import com.oracle.truffle.api.frame.MaterializedFrame;
@@ -107,7 +106,6 @@ public final class PythonLanguage extends TruffleLanguage<PythonContext> {
107106

108107
public Assumption singleContextAssumption = Truffle.getRuntime().createAssumption("Only a single context is active");
109108

110-
@CompilationFinal private boolean nativeBuildTime = TruffleOptions.AOT;
111109
private final NodeFactory nodeFactory;
112110
public final ConcurrentHashMap<Class<? extends PythonBuiltinBaseNode>, RootCallTarget> builtinCallTargetCache = new ConcurrentHashMap<>();
113111

@@ -130,7 +128,6 @@ protected void finalizeContext(PythonContext context) {
130128

131129
@Override
132130
protected boolean patchContext(PythonContext context, Env newEnv) {
133-
nativeBuildTime = false; // now we're running
134131
ensureHomeInOptions(newEnv);
135132
PythonCore.writeInfo("Using preinitialized context.");
136133
context.patch(newEnv);
@@ -458,10 +455,6 @@ private static Source newSource(PythonContext ctxt, SourceBuilder srcBuilder, St
458455
return newBuilder.build();
459456
}
460457

461-
public boolean isNativeBuildTime() {
462-
return nativeBuildTime;
463-
}
464-
465458
@Override
466459
protected void initializeMultipleContexts() {
467460
super.initializeMultipleContexts();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/Python3Core.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
import java.util.ServiceLoader;
4343
import java.util.function.Supplier;
4444

45+
import org.graalvm.nativeimage.ImageInfo;
46+
4547
import com.oracle.graal.python.PythonLanguage;
4648
import com.oracle.graal.python.builtins.modules.ArrayModuleBuiltins;
4749
import com.oracle.graal.python.builtins.modules.AstModuleBuiltins;
@@ -144,8 +146,8 @@
144146
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
145147
import com.oracle.truffle.api.RootCallTarget;
146148
import com.oracle.truffle.api.TruffleFile;
147-
import com.oracle.truffle.api.TruffleLanguage.Env;
148149
import com.oracle.truffle.api.TruffleOptions;
150+
import com.oracle.truffle.api.TruffleLanguage.Env;
149151
import com.oracle.truffle.api.nodes.Node;
150152
import com.oracle.truffle.api.nodes.RootNode;
151153
import com.oracle.truffle.api.source.Source;
@@ -359,7 +361,7 @@ private void initializePythonCore() {
359361

360362
@Override
361363
public void postInitialize() {
362-
if (!getLanguage().isNativeBuildTime()) {
364+
if (!TruffleOptions.AOT || ImageInfo.inImageRuntimeCode()) {
363365
initialized = false;
364366

365367
loadFile(__BUILTINS_PATCHES__, PythonCore.getCoreHomeOrFail());

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinFunctions.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
import com.oracle.graal.python.builtins.objects.PNone;
8383
import com.oracle.graal.python.builtins.objects.PNotImplemented;
8484
import com.oracle.graal.python.builtins.objects.bytes.BytesNodes;
85+
import com.oracle.graal.python.builtins.objects.bytes.OpaqueBytes;
8586
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
8687
import com.oracle.graal.python.builtins.objects.bytes.PIBytesLike;
8788
import com.oracle.graal.python.builtins.objects.cell.PCell;
@@ -601,6 +602,12 @@ Object compile(PBytes source, String filename, String mode, Object kwFlags, Obje
601602
return compile(new String(toBytesNode.execute(source)), filename, mode, kwFlags, kwDontInherit, kwOptimize);
602603
}
603604

605+
@Specialization
606+
@TruffleBoundary
607+
Object compile(OpaqueBytes source, String filename, String mode, Object kwFlags, Object kwDontInherit, Object kwOptimize) {
608+
return compile(new String(source.getBytes()), filename, mode, kwFlags, kwDontInherit, kwOptimize);
609+
}
610+
604611
@SuppressWarnings("unused")
605612
@Specialization
606613
@TruffleBoundary

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java‎

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import com.oracle.graal.python.builtins.modules.PosixModuleBuiltinsFactory.ConvertPathlikeObjectNodeGen;
6767
import com.oracle.graal.python.builtins.modules.PosixModuleBuiltinsFactory.StatNodeFactory;
6868
import com.oracle.graal.python.builtins.objects.PNone;
69+
import com.oracle.graal.python.builtins.objects.bytes.OpaqueBytes;
6970
import com.oracle.graal.python.builtins.objects.bytes.PByteArray;
7071
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
7172
import com.oracle.graal.python.builtins.objects.bytes.PIBytesLike;
@@ -100,6 +101,7 @@
100101
import com.oracle.truffle.api.dsl.NodeFactory;
101102
import com.oracle.truffle.api.dsl.Specialization;
102103
import com.oracle.truffle.api.dsl.TypeSystemReference;
104+
import com.oracle.truffle.api.frame.VirtualFrame;
103105
import com.oracle.truffle.api.profiles.ValueProfile;
104106

105107
@CoreFunctions(defineModule = "posix")
@@ -263,8 +265,12 @@ public abstract static class CwdNode extends PythonBuiltinNode {
263265
@TruffleBoundary
264266
@Specialization
265267
String cwd() {
266-
// TODO(fa) that should actually be retrieved from native code
267-
return System.getProperty("user.dir");
268+
if (getContext().isExecutableAccessAllowed()) {
269+
// TODO(fa) that should actually be retrieved from native code
270+
return System.getProperty("user.dir");
271+
} else {
272+
return "";
273+
}
268274
}
269275

270276
}
@@ -275,14 +281,16 @@ public abstract static class ChdirNode extends PythonBuiltinNode {
275281
@TruffleBoundary
276282
@Specialization
277283
PNone chdir(String spath) {
278-
// TODO(fa) that should actually be set via native code
279-
try {
280-
if (Files.exists(Paths.get(spath))) {
281-
System.setProperty("user.dir", spath);
282-
return PNone.NONE;
284+
if (getContext().isExecutableAccessAllowed()) {
285+
// TODO(fa) that should actually be set via native code
286+
try {
287+
if (Files.exists(Paths.get(spath))) {
288+
System.setProperty("user.dir", spath);
289+
return PNone.NONE;
290+
}
291+
} catch (InvalidPathException e) {
292+
// fall through
283293
}
284-
} catch (InvalidPathException e) {
285-
// fall through
286294
}
287295
throw raise(PythonErrorType.FileNotFoundError, "No such file or directory: '%s'", spath);
288296
}
@@ -795,22 +803,44 @@ public static WriteNode create() {
795803
@GenerateNodeFactory
796804
@TypeSystemReference(PythonArithmeticTypes.class)
797805
public abstract static class ReadNode extends PythonFileNode {
798-
@Specialization
799-
@TruffleBoundary
800-
Object read(int fd, long requestedSize) {
806+
@Specialization(guards = "readOpaque(frame)")
807+
Object readOpaque(@SuppressWarnings("unused") VirtualFrame frame, int fd, @SuppressWarnings("unused") Object requestedSize) {
801808
SeekableByteChannel channel = getFileChannel(fd);
802809
try {
803-
long size = Math.min(requestedSize, channel.size() - channel.position());
804-
// cast below will always succeed, since requestedSize was an int,
805-
// and must thus will always be smaller than a long that cannot be
806-
// downcast
807-
ByteBuffer dst = ByteBuffer.allocate((int) size);
808-
getFileChannel(fd).read(dst);
809-
return factory().createBytes(dst.array());
810+
return new OpaqueBytes(doRead(channel, Integer.MAX_VALUE));
810811
} catch (IOException e) {
811812
throw raise(OSError, e.getMessage());
812813
}
813814
}
815+
816+
@Specialization(guards = "!readOpaque(frame)")
817+
Object read(@SuppressWarnings("unused") VirtualFrame frame, int fd, long requestedSize) {
818+
SeekableByteChannel channel = getFileChannel(fd);
819+
try {
820+
byte[] array = doRead(channel, (int) requestedSize);
821+
return factory().createBytes(array);
822+
} catch (IOException e) {
823+
throw raise(OSError, e.getMessage());
824+
}
825+
}
826+
827+
@TruffleBoundary
828+
private static byte[] doRead(SeekableByteChannel channel, int requestedSize) throws IOException {
829+
long size = Math.min(requestedSize, channel.size() - channel.position());
830+
// cast below will always succeed, since requestedSize was an int,
831+
// and must thus will always be smaller than a long that cannot be
832+
// downcast
833+
ByteBuffer dst = ByteBuffer.allocate((int) size);
834+
channel.read(dst);
835+
return dst.array();
836+
}
837+
838+
/**
839+
* @param frame - only used so the DSL sees this as a dynamic check
840+
*/
841+
protected boolean readOpaque(VirtualFrame frame) {
842+
return OpaqueBytes.isEnabled(getContext());
843+
}
814844
}
815845

816846
@Builtin(name = "isatty", fixedNumOfPositionalArgs = 1)
@@ -1026,6 +1056,9 @@ public void run() {
10261056
@TruffleBoundary
10271057
@Specialization
10281058
int system(String cmd) {
1059+
if (!getContext().isExecutableAccessAllowed()) {
1060+
return -1;
1061+
}
10291062
String[] command = new String[]{shell[0], shell[1], cmd};
10301063
try {
10311064
Runtime rt = Runtime.getRuntime();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/SysModuleBuiltins.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public void initialize(PythonCore core) {
104104
builtinConstants.put("byteorder", ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN ? "little" : "big");
105105
builtinConstants.put("copyright", LICENSE);
106106
builtinConstants.put("dont_write_bytecode", true);
107-
if (TruffleOptions.AOT) {
107+
if (TruffleOptions.AOT || !core.getContext().isExecutableAccessAllowed()) {
108108
// cannot set the path at this time since the binary is not yet known; will be patched
109109
// in the context
110110
builtinConstants.put("executable", PNone.NONE);
@@ -149,6 +149,7 @@ public void initialize(PythonCore core) {
149149
}));
150150
builtinConstants.put("graal_python_core_home", PythonOptions.getOption(core.getContext(), PythonOptions.CoreHome));
151151
builtinConstants.put("graal_python_stdlib_home", PythonOptions.getOption(core.getContext(), PythonOptions.StdLibHome));
152+
builtinConstants.put("graal_python_opaque_filesystem", PythonOptions.getOption(core.getContext(), PythonOptions.OpaqueFilesystem));
152153
// the default values taken from JPython
153154
builtinConstants.put("float_info", core.factory().createTuple(new Object[]{
154155
Double.MAX_VALUE, // DBL_MAX

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/bytes/BytesBuiltins.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
5757
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.NormalizeIndexNode;
5858
import com.oracle.graal.python.builtins.objects.iterator.PSequenceIterator;
59+
import com.oracle.graal.python.builtins.objects.list.PList;
5960
import com.oracle.graal.python.nodes.PNodeWithContext;
6061
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
6162
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
@@ -295,7 +296,33 @@ public Object repr(PBytes self,
295296
@Builtin(name = "join", fixedNumOfPositionalArgs = 2)
296297
@GenerateNodeFactory
297298
public abstract static class JoinNode extends PythonBinaryBuiltinNode {
298-
@Specialization
299+
/**
300+
* @param bytes - the parameter is used to force the DSL to make this a dynamic check
301+
*/
302+
protected boolean readOpaque(PBytes bytes) {
303+
return OpaqueBytes.isEnabled(getContext());
304+
}
305+
306+
@Specialization(guards = {"readOpaque(bytes)"})
307+
public Object join(PBytes bytes, PList iterable,
308+
@Cached("create()") SequenceStorageNodes.GetItemNode getItemNode,
309+
@Cached("create()") SequenceStorageNodes.LenNode lenNode,
310+
@Cached("create()") SequenceStorageNodes.ToByteArrayNode toByteArrayNode,
311+
@Cached("create()") BytesNodes.BytesJoinNode bytesJoinNode) {
312+
int len = lenNode.execute(iterable.getSequenceStorage());
313+
if (len == 1) {
314+
// branch profiles aren't really needed, because of the specialization
315+
// happening in the getItemNode on first execution and the assumption
316+
// in OpaqueBytes.isInstance
317+
Object firstItem = getItemNode.execute(iterable.getSequenceStorage(), 0);
318+
if (OpaqueBytes.isInstance(firstItem)) {
319+
return firstItem;
320+
}
321+
}
322+
return join(bytes, iterable, toByteArrayNode, bytesJoinNode);
323+
}
324+
325+
@Specialization(guards = {"!readOpaque(bytes)"})
299326
public PBytes join(PBytes bytes, Object iterable,
300327
@Cached("create()") SequenceStorageNodes.ToByteArrayNode toByteArrayNode,
301328
@Cached("create()") BytesNodes.BytesJoinNode bytesJoinNode) {

0 commit comments

Comments
 (0)