antoniop in OpenACS

"An iframe which has both allow-scripts and allow-same-origin for its sandbox attribute can remove its sandboxing"

See e.g. https://cloud.google.com/blog/products/data-analytics/iframe-sandbox-tutorial

We set in xooauth/tcl/lti-procs.tcl a restrictive default (all sandboxing restrictions are applied by default). Users should relax it according to their embedded application.

xooauth/www/admin/lti-test.tcl is not really a productive file, so we set the already hardcoded value to no-sandboxing and note that this would be appropriate.

download-archive reform

File-Storage used to generate downloaded archives in tgz format, to then switch to zip, more user-friendly, in particular outside the Linux world (See https://openacs.org/forums/message-view?message_id=557561). To ease the transition, a couple of parameters and relative API were introduced that would allow to choose the preferred command one should use. During this reform however, default parameter values in the tcl code became inconsistent with those in the info file. Furthermore, the chosen defaults were set as absolute paths to the executable, which is not friendly to non-linux environments, or other scenarios where the "typical" Linux filesystem structure cannot be assumed (e.g. containers, MacOS...).

The only usage of this parameters/api was in fact in the download-archive vuh. In upstream codebase, no package references this file, not even the file-storage itself. Upon review, one could see that the file would also allow to specify a custom download filename via the path, which could be considered questionable. It would also execute the command in a way that once again assumes some form of Linux environment (e.g. invoking bash).

Save for the ability to customize the archive format and the anti-feature of being able to manipulate the archive filename via the path, the script largely relplicates www/download-zip, in a better shape after a few reforms hinted by e.g. penetration tools.

Given the aformentioned considerations, I have decided to make download-archive a simple redirect to download-zip. Specifying the object_id via the path will keep working, while URLs out there expecting the name to change will not fail, but the name will not be modified. The archive format will from now on be assumed to be zip.

Improve test:

whether the html filter will accept or not a script tag is configuration-dependent. We now enforce that the outcome is consistent with the security check for HTML used in the filter itself.

Fix method signature

Flush the whole key pattern, now that the key can end either in true or false

Fixes locale__test_lang_conn_browser_locale automated test

Manually replace the ":" entity to prevent attempts at disguising "javascript:" links

Replicate injection attempt by penetration tools

Provide facilities to validate against invalid SQL strings

We introduce a new page contract filter and nsf validator called "dbtext". They implement enforcing of a value to be useable in an SQL query. Currently, this means that the value should not contain the NUL character, but the definition may change in the future or become database-specific.

The html contract filter has also be extended to reject the NUL character.

The test suite has been updated/extended to reflect the changes.

Separate form widgets from label and help-text vertically by wrapping them in a div, render form labels in a larger font for better visibility

Whitespace changes

Harden page contracts

Validate as a token also the default coming from _nls_language, ensure the resulting language key is at most 5 chars long (many thanks to Markus Moser for this)

Update external dependencies to use the URN version

Provide a more meaningful error message

Reform of error handling in ad_page_contract when template recursion is detected

A "complaint recursion" happens if a validation error takes place in one of the templates used while rendering the error page (for instance, anything we include in the master template or the master template itself).

Previously, we would give up complaining after 10 recursions were detected. This had the consequence that after 10 attempt, the failing template involved in rendering the complaint would be fed the invalid data we were trying to reject.

Now, we complain and stop the execution as soon as a recursion is detected. The error will be rendered in a very basic way that overrides the templating system, so that we can exit the recursion cycle.

In practice, only malicious page manipulation attempts should be affected by this change.

Fix date validation using new contract features

Close the connection to the EventSource before leaving the page

Some browsers such as current Firefox may complain otherwise

Many thanks to Sebastian Scheder

Improve visualization of test info

Relax test condition:

make sure potential injections are not rendered on the page response.

Extend test suite

when testing FormPage validation, make sure two distinct behaviors are respected:

1) Rejected values that were part of the request are kept into their original form fields so the user can rework them and resubmit

2) Other parts of the page, such as the page title, are NOT influenced by data that faild to validate

Rationale: displaying unvalidated information as part of the response can be interpreted as a page injection.

In current codebase, that the title was changed indicates, that rejected information made its way into the :title object member of the FormPage.

The potential consequences of the FormPage setting unvalidated information into its members depends on a number of factors such as formfield logics, callbacks and proper page quoting (to name a few).

Replace permission::require_write_permission with permission::require_permission across the calendar package

Rationale: permission::require_write_permission assumes the object creator to have write permission on the object. Instead, we should rely on permissions to be set correctly on the calendar. permission::require_write_permission also performs an additional query to retrieve the object creator.

It is unclear whether permission::require_write_permission makes sense at all as an api, but we leave this to a future post-release reform.

    • -3
    • +11
    /openacs-4/packages/calendar/www/cal-item-view.tcl
Move test from acs-kernel to acs-tcl, add remarks

Fix typo

Bring the logics to parse a datetime from lc_time_fmt into an own private utility lc_datetime_to_clock and reuse it also when we convert from one timezone to another

Test cornercase encountered in practice: a user claims a timezone different to that of the system and creates a calendar item in various formats

This test currently fails, because when we convert the time between the user and the system timezone, we are too strict and allow only the "long" timestamp format "%Y-%m-%d %H:%M:%S"

Allow also dates in the "short" time format, as they may be supplied to the api in such form e.g. by the calendar package

Fixes lang_test__lc_procs automated test

Extend test suite to check that also dates in the "short" time format are supported

This test will fail

Extend test cases

Assume a cal_item is also an acs_object

Qualify source tables in query