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

SubtleCrypto verify methods accepts mutable reference that should be immutable? #3795

Closed
eric-seppanen opened this issue Jan 18, 2024 · 3 comments · Fixed by #3797
Closed
Labels

Comments

@eric-seppanen
Copy link
Contributor

Describe the Bug

Some of the verify methods on SubtleCrypto take mutable references to the signature bytes. It doesn't seem desirable that a method for verifying signatures would mutate the signature bytes, so I wonder if this is a mistake?

Steps to Reproduce

  1. Go to https://docs.rs/web-sys/0.3.67/web_sys/struct.SubtleCrypto.html
  2. Note that some of the verify____ methods accept signature: &mut [u8]

Expected Behavior

I would expect that a signature verification would never need to mutate the signature.

Actual Behavior

Parameter is explicitly set to be mutable; git history doesn't indicate a reason why.

Additional Context

The MDN docs don't suggest that the signature might be mutated. They don't promise it won't, so I'm just going on instinct here.

@eric-seppanen
Copy link
Contributor Author

I notice a few other uses of &mut in SubtleCrypto methods that seem suspicious, e.g. verify_with_xxx_and_yyy_and_u8_array methods accept data: &mut [u8]. This refers to the data over which a signature should be verified; mutating the data would be quite unexpected.

@daxpedda
Copy link
Collaborator

See The wasm-bindgen Guide on "Type Translations in web-sys".

We can add exceptions here:

pub(crate) static IMMUTABLE_SLICE_WHITELIST: Lazy<BTreeSet<&'static str>> = Lazy::new(|| {
BTreeSet::from_iter(vec![
// ImageData
"ImageData",
// WebGlRenderingContext, WebGl2RenderingContext
"uniform1fv",
"uniform2fv",
"uniform3fv",
"uniform4fv",
"uniform1iv",
"uniform2iv",
"uniform3iv",
"uniform4iv",
"uniformMatrix2fv",
"uniformMatrix3fv",
"uniformMatrix4fv",
"uniformMatrix2x3fv",
"uniformMatrix2x4fv",
"uniformMatrix3x2fv",
"uniformMatrix3x4fv",
"uniformMatrix4x2fv",
"uniformMatrix4x3fv",
"vertexAttrib1fv",
"vertexAttrib2fv",
"vertexAttrib3fv",
"vertexAttrib4fv",
"bufferData",
"bufferSubData",
"texImage2D",
"texSubImage2D",
"compressedTexImage2D",
// WebGl2RenderingContext
"uniform1uiv",
"uniform2uiv",
"uniform3uiv",
"uniform4uiv",
"texImage3D",
"texSubImage3D",
"compressedTexImage3D",
"clearBufferfv",
"clearBufferiv",
"clearBufferuiv",
// WebSocket
"send",
// WebGPU
"setBindGroup",
"writeBuffer",
"writeTexture",
// AudioBuffer
"copyToChannel",
// FontFace
"FontFace", // TODO: Add another type's functions here. Leave a comment header with the type name
// FileSystemSyncAccessHandle and FileSystemWritableFileStream
"write",
])
});

I'm happy to review a PR adding exceptions to any SubtleCrypto methods.

@eric-seppanen
Copy link
Contributor Author

Thanks @daxpedda. I investigated all the SubtleCrypto methods and I believe they can all take shared references.

I opened PR #3797 making the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants