Adrian,

Thanks. My apologies for not stating 'NOT'.

Regards,

Pierre

Op 27 april 2012 14:34 schreef Adrian Crum <
[email protected]> het volgende:

> Developers should NOT change it back.
>
> If the script location is contained in a context variable, then it is
> possible for a user to change that variable (through a vulnerable web page)
> to execute ANY script.
>
> Overall, script invocations should be hard-coded. Scripts should not be
> invoked from a variable - there is too much risk in doing things that way.
>
> If you need the ability to specify a script location at run-time, then
> wrap the target scripts in service calls and invoke the service via a
> variable.
>
> -Adrian
>
>
>
> On 4/27/2012 1:21 PM, Pierre Smits wrote:
>
>> Adrian,
>>
>> Could you be a bit more explaining as to why developers should change it
>> back? It will help in understanding the underlying issue (the root and
>> cause).
>>
>> Regards,
>>
>> Pierre
>>
>> Op 27 april 2012 13:34 schreef Adrian Crum<
>> adrian.crum@sandglass-**software.com <[email protected]>>
>>  het volgende:
>>
>>  In this commit I changed the<script>  element "location" attribute from a
>>> FlexibleMapAccessor to a String. In other words, I changed the attribute
>>> type from expression to constant. This change was made for security
>>> reasons. Developers - please don't change it back.
>>>
>>> -Adrian
>>>
>>> On 4/27/2012 12:18 PM, [email protected] wrote:
>>>
>>>  Author: adrianc
>>>> Date: Fri Apr 27 11:18:52 2012
>>>> New Revision: 1331355
>>>>
>>>> URL: 
>>>> http://svn.apache.org/viewvc?****rev=1331355&view=rev<http://svn.apache.org/viewvc?**rev=1331355&view=rev>
>>>> <http://**svn.apache.org/viewvc?rev=**1331355&view=rev<http://svn.apache.org/viewvc?rev=1331355&view=rev>
>>>> >
>>>>
>>>> Log:
>>>> Overhauled Mini-language<script>   element. Added a "script" attribute
>>>> so
>>>> it can be used for small inline scripts (scriptlets).
>>>>
>>>> Removed code that hid script engine exceptions in the error list. If a
>>>> script engine throws an exception, it should be passed to the calling
>>>> process, not hidden in an error message list (which might not get
>>>> checked).
>>>>
>>>> Modified:
>>>>     ofbiz/trunk/framework/****minilang/dtd/simple-methods-****v2.xsd
>>>>     ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java
>>>>
>>>> Modified: ofbiz/trunk/framework/****minilang/dtd/simple-methods-****
>>>> v2.xsd
>>>> URL: 
>>>> http://svn.apache.org/viewvc/****ofbiz/trunk/framework/**<http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**>
>>>> minilang/dtd/simple-methods-****v2.xsd?rev=1331355&r1=1331354&****
>>>> r2=1331355&view=diff<http://**svn.apache.org/viewvc/ofbiz/**
>>>> trunk/framework/minilang/dtd/**simple-methods-v2.xsd?rev=**
>>>> 1331355&r1=1331354&r2=1331355&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331355&r1=1331354&r2=1331355&view=diff>
>>>> >
>>>> ==============================****============================**==**
>>>> ==================
>>>> --- ofbiz/trunk/framework/****minilang/dtd/simple-methods-****v2.xsd
>>>> (original)
>>>> +++ ofbiz/trunk/framework/****minilang/dtd/simple-methods-****v2.xsd
>>>> Fri Apr
>>>>
>>>> 27 11:18:52 2012
>>>> @@ -854,31 +854,34 @@ under the License.
>>>>      <xs:element name="script" substitutionGroup="****CallOperations">
>>>>
>>>>          <xs:annotation>
>>>>              <xs:documentation>
>>>> -                Runs an external script (minilang, bsh, groovy) from
>>>> the
>>>> expanded location provided.
>>>> +                Runs an external script or a short inline script
>>>> (scriptlet).
>>>>                  Error messages go on the error list and are handled
>>>> with
>>>> the check-errors tag.
>>>>              </xs:documentation>
>>>>          </xs:annotation>
>>>>          <xs:complexType mixed="true">
>>>> -<xs:attributeGroup ref="attlist.script"/>
>>>> +<xs:attribute type="xs:string" name="location">
>>>> +<xs:annotation>
>>>> +<xs:documentation>
>>>> +                        The script location. The location attribute
>>>> accepts the component:// file location
>>>> +                        protocol. Script functions/methods can be
>>>> invoked by appending a hash (#) and the
>>>> +                        function/method name.
>>>> +&lt;br/&gt;&lt;br/&gt;
>>>> +                        Required if the script attribute is empty.
>>>> Attribute type: constant.
>>>> +</xs:documentation>
>>>> +</xs:annotation>
>>>> +</xs:attribute>
>>>> +<xs:attribute type="xs:string" name="script">
>>>> +<xs:annotation>
>>>> +<xs:documentation>
>>>> +                        A short script (scriptlet). Can be used instead
>>>> of a file.
>>>> +                        The script must be prefixed with the script
>>>> language followed by a colon (&quot;:&quot;).
>>>> +&lt;br/&gt;&lt;br/&gt;
>>>> +                        Required if the location attribute is empty.
>>>> Attribute type: script.
>>>> +</xs:documentation>
>>>> +</xs:annotation>
>>>> +</xs:attribute>
>>>>          </xs:complexType>
>>>>      </xs:element>
>>>> -<xs:attributeGroup name="attlist.script">
>>>> -<xs:attribute type="xs:string" name="location">
>>>> -<xs:annotation>
>>>> -<xs:documentation>
>>>> -                    Script location (component://...)
>>>> -</xs:documentation>
>>>> -</xs:annotation>
>>>> -</xs:attribute>
>>>> -<xs:attribute type="xs:string" name="error-list-name"
>>>> default="error_list">
>>>> -<xs:annotation>
>>>> -<xs:documentation>
>>>> -                    The name of the list in the method environment to
>>>> check for error messages.
>>>> -                    Defaults to "error_list".
>>>> -</xs:documentation>
>>>> -</xs:annotation>
>>>> -</xs:attribute>
>>>> -</xs:attributeGroup>
>>>>      <xs:element name="call-bsh" substitutionGroup="****
>>>> CallOperations">
>>>>
>>>>          <xs:annotation>
>>>>              <xs:documentation>
>>>> @@ -921,8 +924,7 @@ under the License.
>>>>                  Calls another simple-method in the same context as the
>>>> current one.
>>>>                  The called simple-method will have the same environment
>>>> as the calling simple-method,
>>>>                  including all environment fields, and either the event
>>>> or service objects
>>>> -                that the calling simple-method was called
>>>> -                with.
>>>> +                that the calling simple-method was called with.
>>>>              </xs:documentation>
>>>>          </xs:annotation>
>>>>          <xs:complexType>
>>>> @@ -930,7 +932,7 @@ under the License.
>>>>                  <xs:element ref="result-to-field" minOccurs="0"
>>>> maxOccurs="unbounded">
>>>>                      <xs:annotation>
>>>>                          <xs:documentation>
>>>> -                            Used when memory-model=&quot;function&****
>>>> quot;.
>>>>
>>>> Copies the called method fields
>>>> +                            Used when scope=&quot;function&quot;.
>>>> Copies
>>>> the called method fields
>>>>                              to the calling method fields.
>>>>                          </xs:documentation>
>>>>                      </xs:annotation>
>>>>
>>>> Modified: ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java
>>>> URL: 
>>>> http://svn.apache.org/viewvc/****ofbiz/trunk/framework/**<http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**>
>>>> minilang/src/org/ofbiz/****minilang/method/callops/**
>>>> CallScript.java?rev=1331355&****r1=1331354&r2=1331355&view=****diff<
>>>> http://svn.apache.org/**viewvc/ofbiz/trunk/framework/**
>>>> minilang/src/org/ofbiz/**minilang/method/callops/**
>>>> CallScript.java?rev=1331355&**r1=1331354&r2=1331355&view=**diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CallScript.java?rev=1331355&r1=1331354&r2=1331355&view=diff>
>>>> >
>>>> ==============================****============================**==**
>>>> ==================
>>>> --- ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java (original)
>>>> +++ ofbiz/trunk/framework/****minilang/src/org/ofbiz/**
>>>> minilang/method/callops/****CallScript.java Fri Apr 27 11:18:52 2012
>>>>
>>>> @@ -18,84 +18,101 @@
>>>>   ********************************************************************
>>>> *******************/
>>>>  package org.ofbiz.minilang.method.****callops;
>>>>
>>>>
>>>> -import java.util.List;
>>>> -import java.util.Map;
>>>> -
>>>> -import javolution.util.FastList;
>>>> -
>>>>  import org.ofbiz.base.util.****ScriptUtil;
>>>> +import org.ofbiz.base.util.Scriptlet;
>>>> +import org.ofbiz.base.util.****StringUtil;
>>>> +import org.ofbiz.base.util.string.****FlexibleStringExpander;
>>>>  import org.ofbiz.minilang.****MiniLangException;
>>>> +import org.ofbiz.minilang.****MiniLangRuntimeException;
>>>> +import org.ofbiz.minilang.****MiniLangUtil;
>>>> +import org.ofbiz.minilang.****MiniLangValidate;
>>>>  import org.ofbiz.minilang.****SimpleMethod;
>>>> -import org.ofbiz.minilang.method.****ContextAccessor;
>>>>  import org.ofbiz.minilang.method.****MethodContext;
>>>>  import org.ofbiz.minilang.method.****MethodOperation;
>>>>
>>>>  import org.w3c.dom.Element;
>>>>
>>>> -public class CallScript extends MethodOperation {
>>>> +/**
>>>> + * Executes a script.
>>>> + */
>>>> +public final class CallScript extends MethodOperation {
>>>>
>>>>      public static final String module = CallScript.class.getName();
>>>>
>>>> -    private static String getScriptLocation(String combinedName) {
>>>> -        int pos = combinedName.lastIndexOf("#");
>>>> -        if (pos == -1) {
>>>> -            return combinedName;
>>>> -        }
>>>> -        return combinedName.substring(0, pos);
>>>> -    }
>>>> -
>>>> -    private static String getScriptMethodName(String combinedName) {
>>>> -        int pos = combinedName.lastIndexOf("#");
>>>> -        if (pos == -1) {
>>>> -            return null;
>>>> -        }
>>>> -        return combinedName.substring(pos + 1);
>>>> -    }
>>>> -
>>>> -    private ContextAccessor<List<Object>>   errorListAcsr;
>>>> -    private String location;
>>>> -    private String method;
>>>> +    private final String location;
>>>> +    private final String method;
>>>> +    private final Scriptlet scriptlet;
>>>>
>>>>      public CallScript(Element element, SimpleMethod simpleMethod)
>>>> throws
>>>> MiniLangException {
>>>>          super(element, simpleMethod);
>>>> -        String scriptLocation = element.getAttribute("****location");
>>>> -        this.location = getScriptLocation(****scriptLocation);
>>>> -        this.method = getScriptMethodName(****scriptLocation);
>>>> -        this.errorListAcsr = new ContextAccessor<List<Object>>(****
>>>> element.getAttribute("error-****list-name"), "error_list");
>>>> +        if (MiniLangValidate.****validationOn()) {
>>>> +            MiniLangValidate.****attributeNames(simpleMethod, element,
>>>> "location", "script");
>>>> +            MiniLangValidate.****requireAnyAttribute(****simpleMethod,
>>>>
>>>> element, "location", "script");
>>>> +            MiniLangValidate.****constantAttributes(****simpleMethod,
>>>> element, "location");
>>>> +            MiniLangValidate.****scriptAttributes(simpleMethod,
>>>> element,
>>>> "script");
>>>> +            MiniLangValidate.****noChildElements(simpleMethod,
>>>> element);
>>>> +        }
>>>> +        String scriptAttribute = element.getAttribute("script")****;
>>>> +        if (MiniLangUtil.containsScript(****scriptAttribute)) {
>>>> +            this.scriptlet = new Scriptlet(StringUtil.**
>>>> convertOperatorSubstitutions(****scriptAttribute));
>>>>
>>>> +            this.location = null;
>>>> +            this.method = null;
>>>> +        } else {
>>>> +            this.scriptlet = null;
>>>> +            String scriptLocation = element.getAttribute("****
>>>> location");
>>>> +            int pos = scriptLocation.lastIndexOf("#"****);
>>>>
>>>> +            if (pos == -1) {
>>>> +                this.location = scriptLocation;
>>>> +                this.method = null;
>>>> +            } else {
>>>> +                this.location = scriptLocation.substring(0, pos);
>>>> +                this.method = scriptLocation.substring(pos + 1);
>>>> +            }
>>>> +        }
>>>>      }
>>>>
>>>>      @Override
>>>>      public boolean exec(MethodContext methodContext) throws
>>>> MiniLangException {
>>>> -        String location = methodContext.expandString(****
>>>> this.location);
>>>> -        String method = methodContext.expandString(****this.method);
>>>> -        List<Object>   messages = errorListAcsr.get(****
>>>> methodContext);
>>>>
>>>> -        if (messages == null) {
>>>> -            messages = FastList.newInstance();
>>>> -            errorListAcsr.put(****methodContext, messages);
>>>>
>>>> -        }
>>>> -        Map<String, Object>   context = methodContext.getEnvMap();
>>>> -        if (location.endsWith(".xml")) {
>>>> +        if (this.scriptlet != null) {
>>>>              try {
>>>> -                SimpleMethod.runSimpleMethod(****location, method,
>>>>
>>>> methodContext);
>>>> -            } catch (MiniLangException e) {
>>>> -                messages.add("Error running simple method at location
>>>> ["
>>>> + location + "]: " + e.getMessage());
>>>> +                this.scriptlet.executeScript(****
>>>>
>>>> methodContext.getEnvMap());
>>>> +            } catch (Exception e) {
>>>> +                throw new MiniLangRuntimeException(e.****getMessage(),
>>>>
>>>> this);
>>>>              }
>>>> +            return true;
>>>> +        }
>>>> +        if (location.endsWith(".xml")) {
>>>> +            SimpleMethod.runSimpleMethod(****location, method,
>>>> methodContext);
>>>>          } else {
>>>> -            ScriptUtil.executeScript(this.****location, this.method,
>>>> context);
>>>> +            ScriptUtil.executeScript(this.****location, this.method,
>>>>
>>>> methodContext.getEnvMap());
>>>>          }
>>>> -        // update the method environment
>>>> -        methodContext.putAllEnv(****context);
>>>>
>>>> -        // always return true, error messages just go on the error list
>>>>          return true;
>>>>      }
>>>>
>>>>      @Override
>>>>      public String expandedString(MethodContext methodContext) {
>>>> -        return rawString();
>>>> +        return FlexibleStringExpander.****expandString(toString(),
>>>>
>>>> methodContext.getEnvMap());
>>>>      }
>>>>
>>>>      @Override
>>>>      public String rawString() {
>>>> -        return "<script/>";
>>>> +        return toString();
>>>> +    }
>>>> +
>>>> +    @Override
>>>> +    public String toString() {
>>>> +        StringBuilder sb = new StringBuilder("<script ");
>>>> +        if (this.location != null&&   this.location.length()>   0) {
>>>> +            sb.append("location=\"").****append(this.location);
>>>>
>>>> +            if (this.method != null&&   this.method.length()>   0) {
>>>> +                sb.append("#").append(this.****method);
>>>>
>>>> +            }
>>>> +            sb.append("\" ");
>>>> +        }
>>>> +        if (this.scriptlet != null) {
>>>> +            sb.append("scriptlet=\"").****
>>>> append(this.scriptlet).append(****"\"
>>>>
>>>> ");
>>>> +        }
>>>> +        sb.append("/>");
>>>> +        return sb.toString();
>>>>      }
>>>>
>>>>      public static final class CallScriptFactory implements
>>>> Factory<CallScript>   {
>>>>
>>>>
>>>>
>>>>

Reply via email to