From 247acedd7a6b80f656e04ad629a6c3887fad5028 Mon Sep 17 00:00:00 2001
From: Kuitos <kuitos.lau@gmail.com>
Date: Thu, 13 Oct 2022 21:27:03 +0800
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20should=20keep=20the=20property?=
 =?UTF-8?q?=20writable=20while=20it=20only=20have=20setter=20accessor?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/sandbox/__tests__/proxySandbox.test.ts | 49 ++++++++++++++++++++++
 src/sandbox/proxySandbox.ts                |  9 +++-
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/src/sandbox/__tests__/proxySandbox.test.ts b/src/sandbox/__tests__/proxySandbox.test.ts
index 223dcda1b..b670fedbf 100644
--- a/src/sandbox/__tests__/proxySandbox.test.ts
+++ b/src/sandbox/__tests__/proxySandbox.test.ts
@@ -409,6 +409,55 @@ it('native window function calling should always be bound with window', () => {
   expect(proxy.nativeWindowFunction()).toBe('success');
 });
 
+describe('should work well while the property existed in global context before', () => {
+  it('should not write value while the readonly property existed in global context but not in sandbox', () => {
+    Object.defineProperty(window, 'readonlyPropertyInGlobalContext', {
+      value: 123,
+      writable: false,
+      configurable: true,
+    });
+    const { proxy } = new ProxySandbox('readonly-sandbox');
+    proxy.readonlyPropertyInGlobalContext = 456;
+    expect(proxy.readonlyPropertyInGlobalContext).toBe(123);
+
+    Object.defineProperty(window, 'readonlyPropertyInGlobalContext', {
+      get() {
+        return '123';
+      },
+      configurable: true,
+    });
+    const { proxy: proxy2 } = new ProxySandbox('readonly-sandbox');
+    proxy2.readonlyPropertyInGlobalContext = 456;
+    expect(proxy2.readonlyPropertyInGlobalContext).toBe(123);
+  });
+
+  it('should write value while the writable property existed in global context but not in sandbox', () => {
+    Object.defineProperty(window, 'readonlyPropertyInGlobalContext', {
+      value: 123,
+      writable: true,
+      configurable: true,
+    });
+    const { proxy } = new ProxySandbox('readonly-sandbox');
+    proxy.readonlyPropertyInGlobalContext = 456;
+    proxy.readonlyPropertyInGlobalContext = 789;
+    expect(proxy.readonlyPropertyInGlobalContext).toBe(789);
+
+    Object.defineProperty(window, 'readonlyPropertyInGlobalContext', {
+      get() {
+        return '123';
+      },
+      set() {
+        // do nothing
+      },
+      configurable: true,
+    });
+    const { proxy: proxy2 } = new ProxySandbox('readonly-sandbox');
+    proxy2.readonlyPropertyInGlobalContext = 456;
+    proxy2.readonlyPropertyInGlobalContext = 789;
+    expect(proxy2.readonlyPropertyInGlobalContext).toBe(789);
+  });
+});
+
 describe('variables in whitelist', () => {
   it('should restore window properties (primitive values) that in whitelisted variables', () => {
     const original = {
diff --git a/src/sandbox/proxySandbox.ts b/src/sandbox/proxySandbox.ts
index a9aad44f0..053b96b5b 100644
--- a/src/sandbox/proxySandbox.ts
+++ b/src/sandbox/proxySandbox.ts
@@ -183,8 +183,13 @@ export default class ProxySandbox implements SandBox {
           // We must keep its description while the property existed in globalContext before
           if (!target.hasOwnProperty(p) && globalContext.hasOwnProperty(p)) {
             const descriptor = Object.getOwnPropertyDescriptor(globalContext, p);
-            const { writable, configurable, enumerable } = descriptor!;
-            Object.defineProperty(target, p, { configurable, enumerable, writable, value });
+            const { writable, configurable, enumerable, set } = descriptor!;
+            // only writable property can be overwritten
+            // here we ignored accessor descriptor of globalContext as it makes no sense to trigger its logic(which might make sandbox escaping instead)
+            // we force to set value by data descriptor
+            if (writable || set) {
+              Object.defineProperty(target, p, { configurable, enumerable, writable: true, value });
+            }
           } else {
             target[p] = value;
           }