dcplusplus-devel

[dcplusplus-devel] new lines in hub titles

[poy <po...@12...>]

in ADCH++, it's pretty easy to add a new line to the hub name or 
description, simply by adding a new line in the settings file in the 
contents of the "HubName" or "Description" tags.
this violates the ADC spec ("code point equal to or greater than 32") and 
results in messed up title bar and tab title in DC++.

there are 3 places where behaviors could/should be changed:
- ADCH++ should make sure it doesn't send forbidden chars in its INF.
- DC++ should handle bad chars in incoming INFs and either discard theses 
INFs or replace bad chars with eg spaces.
- DWT's window and tab controls obviously don't support new lines, so they 
should replace CRs & LFs with eg spaces.

poy 

Re: [dcplusplus-devel] new lines in hub titles

[Jan V. K. <jan...@ex...>]

On 8/13/08, poy <po...@12...> wrote:
> in ADCH++, it's pretty easy to add a new line to the hub name or
>  description, simply by adding a new line in the settings file in the
>  contents of the "HubName" or "Description" tags.
>  this violates the ADC spec ("code point equal to or greater than 32") and
>  results in messed up title bar and tab title in DC++.

I think it is OK if it is sent escaped (\n), otherwise it is a weird
INF followed with a garbage command (which would also be exploitable
by allowing injected commands).

The client implementation needs to filter newlines if it cannot
present it "correctly" in the title area or otherwise. The hub is
pretty much at liberty to send whatever it likes anyway, and client
implementations need to take that into consideration.

On the other hand, when it comes to other parts of INF messages, such
as nick names, I think it would be more useful for hub implementations
to enforce stricter rules, such as nicks consisting of (only) white
space. But that is user generated content, not hub created.

-- 
Jan Vidar Krey

Re: [dcplusplus-devel] new lines in hub titles

[poy <po...@12...>]

> I think it is OK if it is sent escaped (\n), otherwise it is a weird
> INF followed with a garbage command (which would also be exploitable
> by allowing injected commands).

ADCH++ does correctly escape commands it sends; there is not the problem. 
but it shouldn't send any \n (new line) or \s (space), or any other char 
with a code point < 32 in the NI (hub name) field of its INF; and no \n (new 
line) or any other char with a code point < 32 in the DE (hub description) 
field of its INF.

related quote from the spec:
NI   | string  | Nickname (or hub name). The hub must ensure that this is 
unique in the hub up to case-sensitivity. Valid are all characters in the 
Unicode character set with code point above 32, although hubs may limit this 
further as they like with an appropriate error message.
DE   | string  | Description. Valid are all characters in the Unicode 
character set with code point equal to or greater than 32.

even if '\' and 's' are above 32, they are in another layer; the char they 
translate into (space - code point 32) isn't allowed in nicks (and by 
extension, in the hub name since it also uses NI).
same for the hub description about new lines (code points 10 or 13) and 
these other chars with a code point < 32.

poy 

Re: [dcplusplus-devel] new lines in hub titles

[Jacek S. <arn...@gm...>]

> there are 3 places where behaviors could/should be changed:
And all three should check I guess...

> - ADCH++ should make sure it doesn't send forbidden chars in its INF.
During startup probably...

> - DC++ should handle bad chars in incoming INFs and either discard theses 
> INFs or replace bad chars with eg spaces.
I think it might actually make more sense to set the name / invalid field to a dummy string like 
"Invalid value" - that'll do more to convince owners not to put these invalid values there in the 
first place...


> - DWT's window and tab controls obviously don't support new lines, so they 
> should replace CRs & LFs with eg spaces.
Hm, I'm not sure this is dwt's responsibility - it should maybe be noted in the docs, but dwt itself 
should safely assume that it's only being passed valid strings...at least if it only results in ugly 
display (not crash)...

And poy is right - the escaping mechanism of ADC works on a lower level and has nothing to do with 
the restrictions placed on the various inf fields.


> poy 

Re: [dcplusplus-devel] new lines in hub titles

[poy <po...@12...>]

forgot the patches...

> patches attached, so that when forbidden chars are encountered:
> - DC++ replaces the name with "InvalidName" and the description with 
> "InvalidDescription".
> - ADCH++ displays a log message and uses the default settings values.
> 
> poy

Re: [dcplusplus-devel] new lines in hub titles

[poy <po...@12...>]

>> - ADCH++ should make sure it doesn't send forbidden chars in its INF.
> During startup probably...
>
>> - DC++ should handle bad chars in incoming INFs and either discard theses 
>> INFs or replace bad chars with eg spaces.
> I think it might actually make more sense to set the name / invalid field 
> to a dummy string like "Invalid value" - that'll do more to convince 
> owners not to put these invalid values there in the first place...

patches attached, so that when forbidden chars are encountered:
- DC++ replaces the name with "InvalidName" and the description with 
"InvalidDescription".
- ADCH++ displays a log message and uses the default settings values.

poy