9:03 p.m.
Hi all
I will add this text to the wiki and upload the wrapper code. Is it
OK to put the code (structure below) in a subdir of each supported
library? Any suggestions as to name? "amiga_lib"?
I suspect the makefile/code can be cleaned up a bit, and possibly
integrated somewhat with the core library buildsystem - but I haven't
investigated as makefiles tend to break if I attempt to change them.
AmigaOS libraries have been created for the main NetSurf project
libraries:
libwapcaplet (wapcaplet.library)
libhubbub (hubbub.library)
libnsbmp (nsbmp.library)
libnsgif (nsgif.library)
libparserutils (parserutils.library)
libcss (css.library)
These comprise of the library itself and a stub C link library, which
opens the AmigaOS library and redirects function calls to the correct
place, negating the need to add additional code to applications to use
it.
Source structure
include/ Includes for building the library
interfaces/ and using without the stub link library
libname.h FUNCTION TABLE
proto/
libname.h
init.c Library init code
makefile.lib Makefile (named so as not to confuse core build system)
libname.library_rev.* Version numbering
stubs/ Stub link library
auto.c Code to open the .library
funcs.c STUB FUNCTIONS
vectors.c FUNCTION TABLE
Building the library
Build the static version of the library as usual:
gmake COMPONENT_TYPE=lib-static BUILD=release
Then cd to the directory containing the AmigaOS library code and run:
make -fmakefile.lib revision (bumps the library revision number)
make -fmakefile.lib (builds the library)
make -fmakefile.lib libname.a (builds the stub link library)
Adding new functions
If a new function is added to the original library, the AmigaOS
library wrapper needs to be updated in the three files marked in
capitals above.
New functions *must* be added to the end of the two function tables
(they must match in both tables, if one is missing from either table
the library will crash unexpectedly!), and a stub function added.
The version numbers in makefile.lib and in the OpenLibrary() call in
stubs/auto.c *must* be increased (this can wait until the next release
of the library/NetSurf - it only needs to be increased if new
functions are added between releases)
Modified API
If the API of a function is modified, it will need to be added as per
a new function in the instructions above. The previous incarnation of
the function will need to be renamed in the two function tables, and
the stub library function amended so it will still work as expected
from old code.
Major incompatible API changes may need a different approach (such as
a "v2" main interface, or a brand new .library with a different name)
Caveats
Amiga libraries have global shared data, rather than per-instance
data. This means some minor modifications have had to be made (within
the AmigaOS library interface, not the original link libraries):
_initialise and _finalise
These routines are called within the library initialisation/exit
code, rather than by the caller. This ensures the library is only
initialised once, and in a consistent state for all users of that
library.
parserutils.library
The initialisation code is hard-coded to read a central aliases file
(L:CharSets/Aliases). Any users of this library needing a custom
aliases file will fail (or the file will need to be merged with the
central one). Such usage is expected to be extremely rare.
wapcaplet.library
There is a security flaw in this library due to the implementation
and its translation to a system global context. All users of this
library get the same context and thus, the same linked-list set of
strings. This has the advantage of greater efficiency, and is
necessary so both (eg) css.library and NetSurf can share strings. Any
sensitive data stored in lwc_strings can easily be extracted by a
malicious user of the library. The nature of wapcaplet.library does
not lend itself to storage of personal information, so it is not
expected that any application will intentionally do this. As
wapcaplet.library could easily be patched to intercept strings as they
are being set, and - in a system without memory protection - any
application can read strings from memory regardless, this additional
problem is deemed insignificant. The warning "do not use
wapcaplet.library for sensitive data" should be heeded (instead link
libwapcaplet.a, or better yet use something more appropriate). This
will be revisited if it is deemed a significant risk.
Regards
Chris
12:35 a.m.
On Wed, Nov 24, 2010 at 09:03:24PM +0000, Chris Young wrote:
wapcaplet.library
There is a security flaw in this library due to the implementation
and its translation to a system global context. All users of this
library get the same context and thus, the same linked-list set of
strings. This has the advantage of greater efficiency, and is
necessary so both (eg) css.library and NetSurf can share strings. Any
sensitive data stored in lwc_strings can easily be extracted by a
malicious user of the library. The nature of wapcaplet.library does
not lend itself to storage of personal information, so it is not
expected that any application will intentionally do this. As
wapcaplet.library could easily be patched to intercept strings as they
are being set, and - in a system without memory protection - any
application can read strings from memory regardless, this additional
problem is deemed insignificant. The warning "do not use
wapcaplet.library for sensitive data" should be heeded (instead link
libwapcaplet.a, or better yet use something more appropriate). This
will be revisited if it is deemed a significant risk.
I may be missing something, but how is this an issue at all? Do all
AmigaOS processes share the same context pointer or similar?
On other OSes, each process gets its own "copy" of the library, and
other processes can't reach the data a process has stored. If this
isn't true, the security issue is with the OS, not libwapcaplet.
B.
6:08 p.m.
On Thu, 25 Nov 2010 00:35:59 +0000, Rob Kendrick wibbled on for an age:
On Wed, Nov 24, 2010 at 09:03:24PM +0000, Chris Young wrote:
> wapcaplet.library
>
> There is a security flaw in this library due to the implementation
> and its translation to a system global context. All users of this
> library get the same context and thus, the same linked-list set of
> strings. This has the advantage of greater efficiency, and is
> necessary so both (eg) css.library and NetSurf can share strings. Any
> sensitive data stored in lwc_strings can easily be extracted by a
> malicious user of the library. The nature of wapcaplet.library does
> not lend itself to storage of personal information, so it is not
> expected that any application will intentionally do this. As
> wapcaplet.library could easily be patched to intercept strings as they
> are being set, and - in a system without memory protection - any
> application can read strings from memory regardless, this additional
> problem is deemed insignificant. The warning "do not use
> wapcaplet.library for sensitive data" should be heeded (instead link
> libwapcaplet.a, or better yet use something more appropriate). This
> will be revisited if it is deemed a significant risk.
I may be missing something, but how is this an issue at all? Do all
AmigaOS processes share the same context pointer or similar?
Yes.
On other OSes, each process gets its own "copy" of the
library, and
other processes can't reach the data a process has stored. If this
isn't true, the security issue is with the OS, not libwapcaplet.
Actually it's just down to the differences between C-style libraries
and .libraries. A .library exists once in memory (code+data), so any
callers will share the same global variables. (the security flaw
isn't with libwapcaplet per se, only this .library version)
Consider a simple example:
int a = 0;
void setvalue(int b)
{
a = b;
}
This is linked into program A and program B as a C-style libexample.a.
If program A calls setvalue(5) this will not affect program B, which
has its own copy of libexample and global variable "a".
However, if this was an example.library, when program A calls
setvalue(5), program B - assuming a corresponding getvalue() function
- would get this value back.
.libraries tend to have an Alloc() function which returns a pointer
which can then be passed to any functions that needs it, rather than
having this as a globally accessible function. In the context of
wapcaplet.library, lwc_initialise would need to return a pointer to
the context, which would then need to be passed to any lwc function
which needs a context pointer - which is only lwc_intern_string(), as
the lwc_string structure could easily be expanded to hold the context
pointer which that string has been created in.
This would obviously need to be cascade down to any calling functions,
and lwc_finalise will need to free the context once finished.
If it's only being used for public things like CSS property names then
leaving it with one global context isn't an issue, and should actually
be beneficial as the data will be shared with multiple programs (which
was of course what set the alarms bells off in the first place)
The other solution is to change back to using .so, but I was trying to
move away from them as they cause problems, have a slight overhead and
don't work properly on OS4.0.
Chris
11:35 p.m.
On Thu, 2010-11-25 at 18:08 +0000, Chris Young wrote:
Actually it's just down to the differences between C-style
libraries
and .libraries. A .library exists once in memory (code+data), so any
callers will share the same global variables. (the security flaw
isn't with libwapcaplet per se, only this .library version)
Aha. In which case, yes, that makes much more sense.
The other solution is to change back to using .so, but I was trying
to
move away from them as they cause problems, have a slight overhead and
don't work properly on OS4.0.
If you're willing to wait a week or so, it's possible that we'll have
time to change the way the libraries work so that they do operate in a
context. This change needs to happen so that they can be used from other
shared libraries, anyway.
Additionally (and with precisely zero knowledge of the issues involved)
it seems to me that the stubs could be auto-generated. Is this correct?
If so, it's probably possible to include the appropriate support in the
core buildsystem.
J.
5:49 p.m.
On Thu, 25 Nov 2010 23:35:33 +0000, John-Mark Bell wrote:
If you're willing to wait a week or so, it's possible that
we'll have
time to change the way the libraries work so that they do operate in a
context. This change needs to happen so that they can be used from other
shared libraries, anyway.
Sure, that's no problem at all. If the change is due to be made
anyway, so much the better.
Additionally (and with precisely zero knowledge of the issues
involved)
it seems to me that the stubs could be auto-generated. Is this correct?
Yes, the only issue would be extracting all the function names from
the header files (and making sure they are always in the same order,
with anything new added to the end), but that's easy to solve, I
think.
If so, it's probably possible to include the appropriate support
in the
core buildsystem.
I'll send you the files privately so you can take a look at them.
Regards
Chris
3819
days inactive
3821
days old
netsurf-dev@netsurf-browser.org
4 comments
3 participants
participants (3)
-
Chris Young -
John-Mark Bell -
Rob Kendrick