Skip to content

Bug 14829 #1398

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

Closed
wants to merge 82 commits into from
Closed

Bug 14829 #1398

wants to merge 82 commits into from

Conversation

mmoqui
Copy link
Member

@mmoqui mmoqui commented Apr 10, 2025

Fix the vulnerability by:

  • removing the feature of changing his own password through the login page,
  • when resetting his password, sending back the same information page for an invalid login id or a valid login id.

silveruser and others added 30 commits June 17, 2024 11:11
The input stream on the body of the Http request is buffered only in the
case of a web entity is carried within it (JSON or XML entity). For any
others data type, no bufferization if done.
Beside that, when buffered, ensure the all the attributes from the
decorated HttpRequest are correctly copied.
…e for development iteration of next version 6.4.2
the regexp to detect potential SQL injections, we can have false
positive. This fix is to involve some regexp for SQL injections so that
they can be more accurate to identify a possible injection attack and
hence to reduce the false positives.
in its scope: componentType. __isComponentType is renamed to
__componentTypeChecker accepting as argument the variable componentType
and then returns a function working on this variable.
datetime are created isn't taken into account when asserting the period
of validity of a variable, despite the datetimes in a Period object are
stored, and hence returned, in UTC. Fix this.
asynchronously and in come circumstances the
completedOptions.store.deferred can be not set when the callback on the
event 'storage:end:store' is executed (in this case, the callback on the
'storage:start:store'  event isn't triggered before as expected).
In the backoffice, the VueJS component driving the management view of
the spaces and components expects the current app instance id to be set
when the view body is on the rendering of a given app instance.

But, in the case of a creation, update or rank move of an app instance,
once the operation done, when the control flow is passed to render the
app instance info page, the current app instance id isn't set and then
the VueJS component redirect the rendering of the view body to the
parent space info page instead.

In order to render the app instance info page in the context of these
operations, it is required to redirect to a switcher: a web page in
which the control flow is passsed to a JS component controlling the
backoffice view (spAdminWindow) that will then update indirectly the
VueJS component and hence the view body and space/app navigation tree.
checked, the password that was effectively checked is cached under an
autogenerated key that is sent back to the client that asked for check.
The client, in the case the password satisfied all the password rules,
sends to the server the check key with the password to save.
If the key doesn't match any cache entry or if the password in the cache
(hence the checked password) doesn't match the password to save, then an
Authentication exception is thrown.
The SilverpeasBeanDAO interface allows to pass directy an SQL where
clause to filter the beans to get or to remove. This is an open door to
SQL injection and we cannot account to the UI or to the service layer to
check the where clause. Some of the where clauses come from directly the
web layer and as such a user can rewrite the clause before this one is
sent to Silverpeas.

In order to avoid this vulnerability, the direct where clause is
replaced by a BeanCriteria that imposes the business layers to decode
the expectations coming from the web layer into a BeanCriteria before
passing it to the SilverpeasBeanDAO object. The BeanCriteria object
represents all the criteria the beans to get or to remove have to
satisfy.

Because, the value of a criterion can be a subquery dedicated to
attempt an attack, BeanCriteria uses a PreparedStatement in which
the parameters are set by using the corresponding interface of the
PreparedStatement. This would prevent such attempts because it encodes
the textual values.
The search by a form is performed against the indexes. When a field of a
form is of type PdC, the field is valued with one or more positions on
the PdC by using two differents separators (one to separate two values
of a given position and another one to separate two different
positions) and this value is then indexed/searched under the index
attribute '<name of the form>##Positioning'. Hence, the search on a such
field is done by looking for indexes with such an attribute having a
value matching exactly the one searched. This means the index entries
in which the attribute contains at least the searched value aren't
returned by the Lucene search engine.

In order to fix this, for the PdC field, the value to search is
rewritten so that to be a regexp understandable by Lucene. In order to
centralize this rewritting, all the code building a QueryDescription for
a search by a form (coming from an XML publication template) is now
centralized into the QueryParameters object. Hence this object is used
both by the search on the PdC (by a search form of a publication
template), and by the Users Directory component.

Fix a bug in the search on WYSYWYG contents of a form field. The bug
comes from a mismatch between the name of the component managing the
users in Silverpeas (and used by the indexation) and the name of the
Directory component when using the publication templates to define
additional information about the users. Now, the name of both is "users"
and this name is used both by the user indexation and by the user
directory. For doing, a migration step for the FormTemplate component
has been defined.

Take the occasion also to improve some codes (as reported by
SonarCloud).
* the user is an administrator or a component manager
* in the app, it can ask to modify it
* in the app admin page, when he validates his changes, a javascript
  error is raised blocking the loading of the admin page in readonly
mode.

Now, according to where the user is (either in the backoffice or in the
front office), once the app parameterizing is done, the view is updated
to the app admin page in readonly mode.
In anonymous mode, the redirection to the contribution referred by a
notification wasn't done correctly when the user isn't authenticated:
* if the user isn't connected, it recieves an access forbidden (normal
  as it is not yet connected)
* and once he signs in, he's redirected into the main page instead of
  the contribution.

Now, when the user accesses from the notification (email) to Silverpeas,
he's automatically redirected into the login page for authentication.
Once authenticated, he's redirected into the page in which is displayed
the contribution referred by the notification.
For doing, in the authentication process, in the session is opening,
the computation of a possible redirection URL takes into account now of
the parameters of redirection. Those parameters are set by
AutoRedirectServlet when the user accesses first, from the notification
(email), to Silverpeas. In our case, this is this servlet that redirects
the user to the login page.

In anonymous mode, if the contribution targeted by a notification is
protected, the user is first automatically redirected to the login page
for authentication, as described above. Some times the protection of the
contribution comes from its parent container (for example, a topic with
specific rights for a publication in a Kmelia): take into account these
peculiarities.
…th of the link starting at the Silverpeas web context
Only chat between users for whom the chat is enabled can be done now.
Obviously, chatting between two users is possible if they are both
connected :-)
As a reminder, a chat is enabled for a user if the following conditions
are met:
* the chat subsystem is enabled
* the domain in which the user belongs is mapped to a XMPP domain
* the user belongs to a group allowed for chatting
The feature regression came in the migration of the chatting client
from JSXC to ConverseJS. Whereas in JSXC we had to implement ourself
the possibility to chat with a non-buddy user (a user not in the
roster), this feature in ConverseJS is already implemented but not
enabled by default. It can be enabled by setting accordingly the
client initialization parameter 'allow_non_roster_messaging'.
…ort to the placeholder attribute to all the form input elements
…efined by the vuejs mixin VuejsFormInputMixin in silverpeas-vue.js
…mulate the time spent by a method treatement: replace the atLeast call by the pollDelay one

Delete the empty test LoggerAnnotationIT.java
…hor and the current date as the creation date when saving the comment in database
mmoqui and others added 26 commits December 10, 2024 16:15
LocalizationBundle#getStringWithParams are now encoded for HTML when
they are String objects. Take into account of this new capabilities in
Silverpeas. This means the HTML specific characters in the parameters
(like &, > or < for examples) will be encoded in their entity number
counterpart (&amp;, &gt; or &lt; in our examples); this shouldn't cause
any issue because the destination of such localized messages are
dedicated to be rendered in HTML (even for emails).

In Web messages, in order to avoid to encode for HTML all the message
text, improve the MessageNotifier#format private static method to apply
the encoding for HTML only on String parameters as those are usually
passed by the user himself when authoring or editing a contribution.

In user notifications, encode any possible text passed by a user in
Silverpeas (the title and the description of a contribution for
example). Take care of the difference between the direct notification
about a contribution by a user and the user notification mechanism when
a contribution is modified or created.
…e for development iteration of next version 6.4.3
Add the onUnload javascript event to the BodyPart tag.
workflow components is now taken in charge. The bundle resources for
those components have to be located, respectively, into the
$SILVERPEAS_HOME/properties/xmlcomponents/personals and
$SILVERPEAS_HOME/properties/xmlcomponents/workflows directories (where
$SILVERPEAS_HOME is the home directory of the Silvepreas installation).
In uploadFile.jsp, encode for HTML the browsbar info in the breadcrumbs.

In the tree menu building, the tree filter implementations are now
retrieved by CDI and not anymore through the classloader.

Fix the code of the tree menu building to please Sonar.

Fix the bug: the sorting of the subspaces is asked.
Delete the spacesInURL.js js as no code in this file is used.
Fix the js code in the wysiwygToolBar.js file.
Fix cycle dependency between TreeBuilder and TreeHandler.
Upgrade the ConverseJS plugins we use:
- action
- jitsimeet
- and screencast
Because of the modification in the Plugin API of ConverseJS since
version 8, our own plugin silverpeas-muc-invitations has been also
modified to be compliant with the new Plugin API.
Refactor the function handlers for the CredentialsServlet servlet in
order they are now CDI managed beans. Use the injection statements for
their dependencies instead of programmatically asking them.
Fix the bug by specifying the id of the sender as system (id -1), so the
email and same properties will be fetched from the
org/silverpeas/notificationserver/channel/smtp/smtpSettings.properties
configuration file.
Fix the vulnerability by setting a restrictive content security policy
of the HTTP response when serving a file in Silverpeas.
Now, when serving a file, it is done explicitly for download and
not anymore for inlining its content.
Improve by simplifying the code of resource sharing by ticket.
Fix the bug: now the publications aliases cannot be shared to other
people for security reasons.

Indeed, the publication aliasing covers two features:
* A way to allow access for publications to users having no access
  for the EDM in which those publications are located.
* A way to multi categorize a publication;
For the first point, for security reasons, the users have only read
rights on the publication aliases. They have then all the functions
mapped to their role that don't break this read-only constraint. Only
users having access for the EDM or the folder in the EDM in which the
alias has been created can then access, through the alias, for the
publication.
For the second point, the multi categorization feature is limited by
the way the publications cannot be directly modified in those different
categorizations; they cannot be modified by their aliases. In fact, the
feature is more a consequence of the first one than a thinkfully one.

Beside this, the folder or publication sharing is a way to share
information both to other users in Silverpeas and external users. This
is why the feature can be a way to counterpass the access constraint on
publications with aliases. So, this fix ensures now the publication
aliases cannot be shared to other people than those expected by the
contributor who created the aliases.
Refactor the credentials management API in order the handlers can
indicate if credentials pre-checking treatment has to be done before
invoking them. The refactor uses the Visitor pattern.

Now, when the user request a password reseting or a password change, if
the pair login/domain doesn't exist in Silverpeas, an error page is
displayed. The HTT response to the navigator doesn't change if there is
an error or not; only the content of the response (the document) change.
The error page instead of talking about the not found login identifier
for a given user domain, informs just the user its password change/reset
isn't allowed.
Now it isn't anymore possible to change his password through the login
form. This feature has been removed for security reasons.
To change his password, the user has either to reset it in the login form
(if this feature is enabled) or to change it in his profile page once
signed in Silverpeas.

When reseting his password with an invalid login id, the same message is
given than with a valid login id. So nobody cannot know if a user with
such a login id exists or not.
@mmoqui mmoqui closed this Apr 10, 2025
@mmoqui mmoqui deleted the bug-14829 branch April 10, 2025 15:22
@mmoqui mmoqui restored the bug-14829 branch April 10, 2025 15:23
@mmoqui mmoqui deleted the bug-14829 branch April 11, 2025 07:16
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.

1 participant