bugSavane - Bugs: bug #678, (CERN) Fix code related to size...

 
 
Show feedback again

You are not allowed to post comments on this tracker with your current authentification level.

bug #678: (CERN) Fix code related to size checking of attached files

Submitted by:  Mathieu Roy <yeupou>
Submitted on:  Mon 06 Sep 2004 05:00:29 PM UTC  
 
Category: NoneStatus: Fixed
Severity: 1 - WishPriority: A - Later
Assigned to: Mathieu Roy <yeupou>Open/Closed: Closed
Release: 1.0.1-CERNPlanned Release: 
Reproducibility: NonePrivacy: Public

Tue 07 Sep 2004 03:19:56 PM UTC, comment #2:

About
http://savannah.cern.ch/bugs/?func=detailitem&item_id=4065

-> It is not a bug that the update is sent even if the attachment failed, since we do not refuse the bug posting and the rest of the submitted data is well registered.

-> strlen() is maybe not very efficient for large files, but what else? BTW, on large files, apache/PHP should drop the request by itself.

-> I believe it is on purpose that the filesize test is made after the addslashes(). Otherwise, why not using only filesize(). It is confusing for users, I'm willing to admit it. But file upload is something very sensitive when it comes to webservers, frequently used for exploit. We're forced to rush addslashes() when inserting data in the database to avoid malicious exploits. But I guess someone could act maliciously if we do filesize checks before the addslashes: someone could forge a file to triple the size after the addslashes() call, so he could upload a file way way bigger than the limit that would pass the check.
So in fact, we should probably explain the reason of the refusal more in details, but not change the test.

The applied solution is to feedback the exact size found, after calling addslashes(), in case of refusal.

Mathieu Roy <yeupou>
Project AdministratorIn charge of this item.
Tue 07 Sep 2004 03:16:20 PM UTC, comment #1:

About
http://savannah.cern.ch/bugs/?func=detailitem&item_id=4065

-> It is not a bug that the update is sent even if the attachment failed, since we do not refuse the bug posting and the rest of the submitted data is well registered.

-> strlen() is maybe not very efficient for large files, but what else? BTW, on large files, apache/PHP should drop the request by itself.

-> I believe it is on purpose that the filesize test is made after the addslashes(). Otherwise, why not using only filesize(). It is confusing for users, I'm willing to admit it. But file upload is something very sensitive when it comes to webservers, frequently used for exploit. We're forced to rush addslashes() when inserting data in the database to avoid malicious exploits. But I guess someone could act maliciously if we do filesize checks before the addslashes: someone could forge a file to triple the size after the addslashes() call, so he could upload a file way way bigger than the limit that would pass the check.
So in fact, we should probably explain the reason of the refusal more in details, but not change the test.

Mathieu Roy <yeupou>
Project AdministratorIn charge of this item.
Mon 06 Sep 2004 05:00:29 PM UTC, original submission:

Fix code related to size checking of attached files
- include/trackers/general.php

Mathieu Roy <yeupou>
Project AdministratorIn charge of this item.

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one

Digest:
   task dependencies.

 

Carbon-Copy List
  • -unavailable- added by yeupou
  • -unavailable- added by yeupou (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 07 Sep 2004 03:19:56 PM UTCyeupouStatusNone=>Fixed
      Open/ClosedOpen=>Closed
      Carbon-Copy-=>Added ype
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup