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

Add a settings to limit the width & height params #78

Open
lionep opened this issue Jan 23, 2024 · 3 comments
Open

Add a settings to limit the width & height params #78

lionep opened this issue Jan 23, 2024 · 3 comments

Comments

@lionep
Copy link

lionep commented Jan 23, 2024

Hello,

Nice project out there !
I've tried it and it seems you can overscale pictures, and get some insufficient memory usage.

Would it be possible to implement :

  • A config params (as environment variable) to prevent overscale (width or height, over original width or height)
  • A config params to define maximum allowed width or height in query params, returning an error if user goes above the value.

Also, it seems that every different size request is stored in e_image_derivative_backend table, so a anon user can fillup the disk with commands like :

for I in `seq 100 500`; do
  curl -v https://picsur.domain.com/i/SOME_PICTURE_ID.jpg\?height\=$I
done

Any idea of a protection agains this ?

Thanks !

@accessiblepixel
Copy link

I've just checked on my instance, and the GUI seems to restrict it to 4 numbers (so 9999 x 9999 would be appear to be the maximum)... Although if you put it in the URL request in it will accept up to 32767 x 32767... Above that it returns an error for the number being too big.

This is a kinda sensible thing to prevent, although I do agree that it would be good to limit upscaling (maybe to 1x or 2x the image, at most) since when I put the request in it exhausted all available memory and the worker returned error 500.

I've found where it's setting that upper limit, in the ``shared/src/dto/api/image.dto.ts" file Here

So I could maybe change the max sizes to being like 4000x4000 or something sensible although I don't know much about typescript or docker to change/build it...

@proegssilb
Copy link

Relevant code that would consume env vars: https://github.com/CaramelFur/Picsur/blob/master/backend/src/managers/image/image-converter.service.ts#L86

Haven't figured out if there's some other place in the code where configuration from env vars is centralized to. Appreciate the dependency injection.

@accessiblepixel
Copy link

Hmm. I tried to build a test with just changing those options I found, but the build process doesn't run entirely locally (at least as far as I can get with it), stage 1 tries to push to github, and then stage2 downloads from that push, so without setting up github and editing it I can't figure out how to test it and then make a good pull request.

At least we know where (some of?) those values are from/used now :)

Seasons greetings :)

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

No branches or pull requests

3 participants