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 potential XSS in the preview button of FileWidget #4065

Merged
merged 8 commits into from
Jan 30, 2024

Conversation

yuki-js
Copy link
Contributor

@yuki-js yuki-js commented Jan 25, 2024

Reasons for making this change

fixes #4057

The value prop in FileWidget parses the data URL inappropriately, which also permits the use of javascript: schema.
When the javascript: schema is set, and ui:filePreview is true, and when the user clicks Preview button, the JavaScript code is evaluated, and then allows XSS attack.

I fixed it by:

  • parsing the data URL correctly
  • throwing Error when the URL is invalid
  • restricting image types from any image to JPEG, PNG, GIF

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

packages/core/src/components/widgets/FileWidget.tsx Outdated Show resolved Hide resolved
packages/utils/src/dataURItoBlob.ts Outdated Show resolved Hide resolved
}
// Create the blob object
const blob = new window.Blob([new Uint8Array(array)], { type });

return { blob, name };
} catch (error) {
return { blob: { size: 0, type: (error as Error).message }, name: dataURI };
throw new Error('File is invalid: failed to decode base64');
Copy link
Member

Choose a reason for hiding this comment

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

Why not just treat all of the errors to return a blob like these?

Copy link
Contributor Author

@yuki-js yuki-js Jan 27, 2024

Choose a reason for hiding this comment

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

The original code treated the invalid file as zero-sized file. Of course, the invalid file is different from the zero-sized valid file. Invalid files should have been taken care of by the callee, but actually it was not.
This behavior also allowed javascript: schema.
I fixed this by handling these errors correctly.

@heath-freenome
Copy link
Member

@yuki-js your build in @rjsf/utils is failing.

@yuki-js
Copy link
Contributor Author

yuki-js commented Jan 27, 2024

@heath-freenome Thank you for your review.
This failure is the same as the one we faced previously. Could you rerun it?
https://github.com/rjsf-team/react-jsonschema-form/actions/runs/7474848380/attempts/3

I'm sorry it was my oversight.

to pass the coverage test
Copy link
Member

@heath-freenome heath-freenome left a comment

Choose a reason for hiding this comment

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

Sorry, I forgot to submit these comments over the weekend

packages/core/src/components/widgets/FileWidget.tsx Outdated Show resolved Hide resolved
},
];
} catch (e) {
// Invalid dataURI, so just ignore it.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we want to report the exception to the user? Or make the file upload report it so that they don't wonder why their file disappeared

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibility of Exception: When the ordinary user uploads files in ordinary way, the exception should never happen, since the data URL is always returned, and Base64 encoding/decoding always be successful. Therefore the data should not disappear in an unintuitive way.

Error notification: If the caller injects the malformed string into formData, validator rejects it with .files.0 must match format "data-url", but it would be an issue on the side of the caller.

Although I could also change it by adding error flag in FileInfoType, I decided not to notify the explicit error for this reason.

yuki-js and others added 2 commits January 30, 2024 15:18
Co-authored-by: Heath C <51679588+heath-freenome@users.noreply.github.com>
@heath-freenome heath-freenome merged commit ee2ac9b into rjsf-team:main Jan 30, 2024
4 checks passed
@yuki-js yuki-js deleted the fix/file-xss branch January 30, 2024 16:28
nickgros added a commit that referenced this pull request Apr 19, 2024
Co-authored-by: Heath C <51679588+heath-freenome@users.noreply.github.com>
Co-authored-by: Abdallah Al-Soqatri <abdallah.al-soqatri@aspentech.com>
Co-authored-by: Kevin Burnett <18027+burnettk@users.noreply.github.com>
Co-authored-by: Marek Bodinger <marek.bodinger@gmail.com>
Co-authored-by: Mehdi Salem <mehdi.salem@qt.io>
Co-authored-by: Jonasz Wiącek <jonaszwiacek@gmail.com>
Co-authored-by: Bogdan Savluk <savluk.bogdan@gmail.com>
Co-authored-by: Christian Wendt <54559756+cwendtxealth@users.noreply.github.com>
Co-authored-by: Ben Lambert <ben@blam.sh>
Co-authored-by: David R. Bild <david@davidbild.org>
Co-authored-by: Ariqun <38001928+Ariqun@users.noreply.github.com>
Co-authored-by: Shivam Anand Murmu <35562703+Rozamo@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Shubham Biswas <46351104+Shubhcoder@users.noreply.github.com>
Co-authored-by: popmanhe <neo_temp@hotmail.com>
Co-authored-by: Yuki Aoki <me@aoki.app>
Co-authored-by: Xiangcheng Kuo <37873394+orange-guo@users.noreply.github.com>
Co-authored-by: Bart van Andel <bavanandel@gmail.com>
Co-authored-by: Laurent Direr <laurent.direr@gmail.com>
Co-authored-by: Vegard Stenvik <42935080+vstenvik@users.noreply.github.com>
Co-authored-by: Appie <abdallarko@hotmail.com>
Co-authored-by: Oren Forer <oforer@gmail.com>
Co-authored-by: Marcus Penn <11893741+mpenndev@users.noreply.github.com>
Co-authored-by: joachimhagheim <47362824+joachimhagheim@users.noreply.github.com>
Co-authored-by: MarekBodingerBA <104828482+MarekBodingerBA@users.noreply.github.com>
Co-authored-by: momesana <momesana@gmail.com>
Co-authored-by: Martti Roitto <MarttiR@users.noreply.github.com>
fix(utils): direct lodash function import to improve bundling on library client side (#3976)
fix: #3961 resolve all recurse list for object properties (#3981)
fix gap in outline when label is hidden (#3984)
Fix: Expose the internal `ajv` variable in the validator implementation classes (#3991)
Fixes: #3972 indirectly by exposing the `ajv` variable for use in the issue
Fix: Change FormHelperText usage with @mui/material to render divs (#4032)
Fixes #4031 by switching the render component for `FormHelperText` to be `div`
fix: Added support for anyOf/oneOf in uiSchema (#4055)
Fixes #4039 by updating `MultiSchemaField` to properly support `anyOf`/`oneOf` arrays in the `uiSchema`
Fix checkbox with 0 as a value was unselectable in antd (#4068)
Fixed #4067 by properly dealing with enums that have 0 as a value
Fix potential XSS in the preview button of FileWidget (#4065)
Fix: Make 'ui:rows' option work with chakra-ui for textarea elements #4070 (#4078)
Fix typo in ErrorsListTemplate example (#4087)
Fix #4080 by moving `base64` encoder/decoder from `@rjsf/utils` to playground (#4093)
Fix: Error state not resetting when schema changes (#4079) (#4103)
Fix noImplicitAny error (#4106)
Fixes: [WARNING] Duplicate key "include" in object literal [duplicate-object-key] (#4114)
Fixes: Warning: validateDOMNesting(...): <p> cannot appear as a descendant of <p>. (#4117)
Fix documentation to add missing Form imports (#4131)
Fix #4127 to add missing `Form` import in documentation
Fix: filename should be bold (#4125)
Fix: use correct ConfigProvider context by using named imports (#4132)
Fix 4134 by filtering out bad DOM props (#4140)
Fixes: #4134 by updating the spreading of props onto the `TextField` to remove bad DOM fields
Fixed Programmatic submit not working properly in Firefox (#4150)
Fix Maximum call stack size exceeded in findSchemaDefinition (#4123)
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.

Validate dataURL to prevent XSS attacks
2 participants