Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use null prototype for LayoutAnimationRepository config #3383

Merged
merged 1 commit into from
Jul 27, 2022

Conversation

matias-la
Copy link
Contributor

@matias-la matias-la commented Jul 12, 2022

Description

Prevent the configs object from having properties inherited from Object.prototype, such as toString or __proto__. Otherwise, using these properties could potentially have a security impact.

Changes

Changed the prototype of the configs object so it doesn't inherit from Object.prototype, as regular JS objects do by default.

Test code and steps to reproduce

Couldn't find a test case that I could base on to demonstrate how this could present a risk.

An example of the problem caused by using an object inheriting from Object.prototype can be demonstrated by the following line (corresponding to the startAnimationForTag function):

const style = configs[tag][type](yogaValues);

If both tag and type had the value constructor, style would have the result of evaluating configs.constructor.constructor(yogaValues), which in many platforms would be the same as new Function(yogaValues). This could potentially be used to create functions with malicious code. Although I suspect the conditions to make this work would be hard to achieve, it's probably better to get rid of this potential threat by using null prototypes.

Checklist

  • Included code example that can be used to test this change
  • Updated TS types
  • Added TS types tests
  • Added unit / integration tests
  • Updated documentation
  • Ensured that CI passes

Prevent the configs object from having properties inherited from
Object.prototype, such as toString or __proto__. Otherwise, using these
properties could potentially have a security impact.
Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for another valuable PR! ❤️

@piaskowyk piaskowyk merged commit 730e3b5 into software-mansion:main Jul 27, 2022
piaskowyk pushed a commit that referenced this pull request Jul 27, 2022
## Description

Prevent the `configs` object from having properties inherited from `Object.prototype`, such as `toString` or `__proto__`. Otherwise, using these properties could potentially have a security impact.

## Changes

Changed the prototype of the `configs` object so it doesn't inherit from `Object.prototype`, as regular JS objects do by default.


## Test code and steps to reproduce

Couldn't find a test case that I could base on to demonstrate how this could present a risk.

An example of the problem caused by using an object inheriting from `Object.prototype` can be demonstrated by the following line (corresponding to the `startAnimationForTag` function):

```js
const style = configs[tag][type](yogaValues);
```

If both `tag` and `type` had the value `constructor`, style would have the result of evaluating `configs.constructor.constructor(yogaValues)`, which in many platforms would be the same as `new Function(yogaValues)`. This could potentially be used to create functions with malicious code. Although I suspect the conditions to make this work would be hard to achieve, it's probably better to get rid of this potential threat by using null prototypes.

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Added TS types tests
- [ ] Added unit / integration tests
- [ ] Updated documentation
- [ ] Ensured that CI passes
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
…e-mansion#3383)

## Description

Prevent the `configs` object from having properties inherited from `Object.prototype`, such as `toString` or `__proto__`. Otherwise, using these properties could potentially have a security impact.

## Changes

Changed the prototype of the `configs` object so it doesn't inherit from `Object.prototype`, as regular JS objects do by default.


## Test code and steps to reproduce

Couldn't find a test case that I could base on to demonstrate how this could present a risk.

An example of the problem caused by using an object inheriting from `Object.prototype` can be demonstrated by the following line (corresponding to the `startAnimationForTag` function):

```js
const style = configs[tag][type](yogaValues);
```

If both `tag` and `type` had the value `constructor`, style would have the result of evaluating `configs.constructor.constructor(yogaValues)`, which in many platforms would be the same as `new Function(yogaValues)`. This could potentially be used to create functions with malicious code. Although I suspect the conditions to make this work would be hard to achieve, it's probably better to get rid of this potential threat by using null prototypes.

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Added TS types tests
- [ ] Added unit / integration tests
- [ ] Updated documentation
- [ ] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants